Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Renamed procs/jobs to steps in code #1331

Merged
merged 25 commits into from
Oct 28, 2022

Conversation

Harikesh00
Copy link
Contributor

@Harikesh00 Harikesh00 commented Oct 24, 2022

Renamed procs to steps in code for the issue #1288

@Harikesh00 Harikesh00 changed the title Renamed procs to steps in code #1288 Renamed procs to steps in code Oct 24, 2022
@Harikesh00
Copy link
Contributor Author

@6543 plz review

Copy link
Contributor

@qwerty287 qwerty287 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two generic infos:

  1. please add some migration information about what changed to the corresponding file in docs
  2. you must add a DB migration that updates table / column names

.gitpod.yml Outdated Show resolved Hide resolved
server/model/step.go Show resolved Hide resolved
server/model/step.go Show resolved Hide resolved
server/store/datastore/migration/000_legacy_to_xorm.go Outdated Show resolved Hide resolved
agent/runner.go Show resolved Hide resolved
agent/runner.go Show resolved Hide resolved
cli/exec/line.go Show resolved Hide resolved
cli/pipeline/ps.go Show resolved Hide resolved
cmd/agent/flags.go Outdated Show resolved Hide resolved
pipeline/rpc/line.go Show resolved Hide resolved
pipeline/tracer.go Outdated Show resolved Hide resolved
server/model/file.go Show resolved Hide resolved
server/store/datastore/migration/000_legacy_to_xorm.go Outdated Show resolved Hide resolved
server/model/log.go Outdated Show resolved Hide resolved
@6543 6543 added the refactor delete or replace old code label Oct 24, 2022
@qwerty287 qwerty287 added this to the 1.0.0 milestone Oct 24, 2022
@woodpecker-bot
Copy link
Contributor

woodpecker-bot commented Oct 24, 2022

Deployment of preview was successful: https://woodpecker-ci-woodpecker-pr-1331.surge.sh

@Harikesh00

This comment was marked as outdated.

@qwerty287
Copy link
Contributor

Depending on the kind of breaking change. If it affects database table/columm names, you're done with a migration. Otherwise it either will be breaking (depending on how important it is that's fine too, but then add a notice to the docs in the migrations file) or you should add a fallback.

@Harikesh00
Copy link
Contributor Author

Harikesh00 commented Oct 26, 2022

ok, adding migration script and keeping the changes related to migration (wherever commented need migration)
let's review, if any issues, will resolve them based on the comments
and don't know what will be the fallback

@Harikesh00
Copy link
Contributor Author

@qwerty287 @6543 migration script added & review comments resolved
plz review

@Harikesh00
Copy link
Contributor Author

I didn't get what fallback means, why it's required and what it will do
can u plz explain

@qwerty287
Copy link
Contributor

If you just change an environment variable, it will break most users config. So we add a fallback to the old variable name (or whatever else has been breaking) to still be usable with older configs. I can look a bit more into this PR and fix some remaining stuff.

@qwerty287
Copy link
Contributor

@6543

With dff5e9e, I introduced some breaking (like JSON) changes again. Is this fine? We did that with build -> pipeline as well.

@qwerty287
Copy link
Contributor

I pushed various commits that finish renaming and fixes existing stuff. @6543 Probably you'll still find something we have to improve, but I don't really see much more.

@qwerty287 qwerty287 changed the title Renamed procs to steps in code Renamed procs/jobs to steps in code Oct 26, 2022
@codecov-commenter
Copy link

codecov-commenter commented Oct 26, 2022

Codecov Report

Base: 44.71% // Head: 44.89% // Increases project coverage by +0.17% 🎉

Coverage data is based on head (91a09fc) compared to base (da7d13a).
Patch coverage: 47.20% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1331      +/-   ##
==========================================
+ Coverage   44.71%   44.89%   +0.17%     
==========================================
  Files         137      138       +1     
  Lines        9873     9941      +68     
==========================================
+ Hits         4415     4463      +48     
- Misses       5196     5213      +17     
- Partials      262      265       +3     
Impacted Files Coverage Δ
cmd/agent/agent.go 0.00% <0.00%> (ø)
pipeline/backend/docker/convert.go 8.73% <0.00%> (ø)
pipeline/backend/docker/docker.go 0.00% <0.00%> (ø)
pipeline/frontend/yaml/compiler/convert.go 0.00% <0.00%> (ø)
pipeline/logger.go 0.00% <ø> (ø)
pipeline/rpc/client_grpc.go 0.00% <0.00%> (ø)
pipeline/tracer.go 0.00% <0.00%> (ø)
server/grpc/rpc.go 0.00% <0.00%> (ø)
server/grpc/server.go 0.00% <0.00%> (ø)
server/model/file.go 0.00% <ø> (ø)
... and 26 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

cmd/agent/flags.go Outdated Show resolved Hide resolved
docs/docs/92-awesome.md Outdated Show resolved Hide resolved
cmd/agent/flags.go Outdated Show resolved Hide resolved
@6543

This comment was marked as outdated.

web/web.go Outdated Show resolved Hide resolved
@qwerty287 qwerty287 mentioned this pull request Oct 28, 2022
server/shared/stepStatus.go Outdated Show resolved Hide resolved
@6543 6543 requested a review from qwerty287 October 28, 2022 13:43
@6543 6543 merged commit 36e4291 into woodpecker-ci:master Oct 28, 2022
@qwerty287
Copy link
Contributor

Thanks @Harikesh00 for the initial work on this.

@6543
Copy link
Member

6543 commented Oct 29, 2022

☝️

@Harikesh00
Copy link
Contributor Author

Thank you @6543 @qwerty287 for all your support, you did a lot

simmstein pushed a commit to simmstein/woodpecker that referenced this pull request Dec 27, 2022
Renamed `procs` to `steps` in code for the issue woodpecker-ci#1288

Co-authored-by: Harikesh Prajapati <harikesh.prajapati@druva.com>
Co-authored-by: qwerty287 <ndev@web.de>
Co-authored-by: qwerty287 <80460567+qwerty287@users.noreply.github.com>
Co-authored-by: 6543 <6543@obermui.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor delete or replace old code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants