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

Remove some ci environment variables #3846

Merged
merged 15 commits into from
Sep 8, 2024

Conversation

6543
Copy link
Member

@6543 6543 commented Jun 26, 2024

remove some environment variables who are either the same value or make no sense to exist in the first place:

CI_STEP_STATUS
CI_STEP_FINISHED
CI_PIPELINE_STATUS
CI_PIPELINE_FINISHED
CI_COMMIT_URL

CI_STEP_STATUS
CI_STEP_FINISHED
CI_PIPELINE_STATUS
CI_PIPELINE_FINISHED
@6543 6543 added the breaking will break existing installations if no manual action happens label Jun 26, 2024
@anbraten
Copy link
Member

Could we remove those things just-in-time when we release 3.0. Those PRs will probably be quite outdated and need updates as we are constantly adding new deprecations til we release 3.0.

@qwerty287
Copy link
Contributor

We should do 3.0 soon I think. There are enough breaing changes and deprecations collected. Maybe after 2.7.0?

@anbraten
Copy link
Member

anbraten commented Jun 27, 2024

We should do 3.0 soon I think. There are enough breaing changes and deprecations collected. Maybe after 2.7.0?

I would like to like to do it earliest end of the year. We should provide the community some time between major releases and the breaking changes we would currently apply aren't that critical IMO and the benefits from a users perspective are rather minimal atm.

@zc-devs
Copy link
Contributor

zc-devs commented Jun 27, 2024

I would add DRONE_* to the list.

@qwerty287
Copy link
Contributor

We keep the drone vars to be compatible to drone plugins.

@6543 6543 added the blocked It's ready but something external is blocking it label Jun 27, 2024
@6543
Copy link
Member Author

6543 commented Jun 27, 2024

... Those PRs will probably be quite outdated and need updates as we are constantly adding new deprecations til we release 3.0.

yes I just want to start adding the changes here and I'll peridically resolve conflicts and update it etc ... as it's easuly forgotten otherwise

@anbraten anbraten added this to the 3.x.x milestone Jul 1, 2024
@qwerty287 qwerty287 modified the milestones: 3.x.x, 3.0.0 Jul 17, 2024
@6543 6543 mentioned this pull request Jul 19, 2024
@6543 6543 marked this pull request as ready for review July 22, 2024 01:01
@6543 6543 changed the title WIP: remove some ci environment variables Remove some ci environment variables Jul 22, 2024
@6543 6543 added refactor delete or replace old code and removed blocked It's ready but something external is blocking it labels Jul 22, 2024
@woodpecker-bot
Copy link
Collaborator

woodpecker-bot commented Jul 22, 2024

@6543 6543 requested a review from a team July 22, 2024 01:17
Copy link

codecov bot commented Jul 22, 2024

Codecov Report

Attention: Patch coverage is 42.85714% with 4 lines in your changes missing coverage. Please review.

Project coverage is 26.98%. Comparing base (f7d12bf) to head (45475e1).
Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
pipeline/frontend/metadata/environment.go 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3846      +/-   ##
==========================================
+ Coverage   26.96%   26.98%   +0.02%     
==========================================
  Files         394      394              
  Lines       27418    27395      -23     
==========================================
  Hits         7393     7393              
+ Misses      19325    19302      -23     
  Partials      700      700              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@6543 6543 mentioned this pull request Jul 22, 2024
1 task
@qwerty287
Copy link
Contributor

Canyou remove CI_COMMIT_URL too? It's deprecated

@6543
Copy link
Member Author

6543 commented Jul 22, 2024

@qwerty287 done

agent/tracer.go Show resolved Hide resolved
docs/docs/91-migrations.md Outdated Show resolved Hide resolved
@lafriks
Copy link
Contributor

lafriks commented Sep 5, 2024

Can't pipeline status be failed if step is executed even when previous steps failed so that you can notify pipeline status?

@6543
Copy link
Member Author

6543 commented Sep 5, 2024

The previous variables are not touched ans are still here ...

@6543
Copy link
Member Author

6543 commented Sep 5, 2024

Can't pipeline status be failed if step is executed even when previous steps failed so that you can notify pipeline status?

well the point is ... a whole pipeline status is completely irrelevant in a step ... enven within the workflow context it is hard to be usefull

and we still have some missmatch because pipeline != workflow but the vars use workflow metadata and prefix it pipeline :/

@6543
Copy link
Member Author

6543 commented Sep 5, 2024

tldr: I dont see any any use in having it in as it is now, if we want something similar it shold be implemented with a new method and with the current terminology in mind e.g. dedicated feature pull

@6543 6543 requested a review from qwerty287 September 6, 2024 09:05
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.

untested

docs/docs/91-migrations.md Outdated Show resolved Hide resolved
@6543 6543 enabled auto-merge (squash) September 7, 2024 23:49
@6543 6543 merged commit 38ed7f9 into woodpecker-ci:main Sep 8, 2024
9 checks passed
@woodpecker-bot woodpecker-bot mentioned this pull request Sep 8, 2024
1 task
@6543 6543 deleted the rm-useles-envs branch September 8, 2024 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking will break existing installations if no manual action happens refactor delete or replace old code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants