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

Update github action versions #22

Merged
merged 1 commit into from
May 23, 2024
Merged

Update github action versions #22

merged 1 commit into from
May 23, 2024

Conversation

hugobuddel
Copy link
Contributor

  • actions/setup-python@v5 updates node version from node16 to node20
  • actions/checkout@v4 seems to be the standard
  • actions/setup-node@v4 recommends specifying a node version

This change is prompted by github complaining that "Node.js 16 actions are deprecated. Please update the following actions to use Node.js 20: actions/setup-python@v4, codecov/codecov-action@v3." E.g: https://github.com/AstarVienna/ScopeSim/actions/runs/9057582692

However, setup-python@v4 still uses node 16 apparently, so v5 is needed. Codecov-action has a v4 but that has breaking changes, so I did not want to upgrade that, as it did not seem necessary.

@hugobuddel hugobuddel requested a review from teutoburg May 13, 2024 07:35
@hugobuddel
Copy link
Contributor Author

The github actions also complain that

[notice] A new release of pip is available: 22.0.4 -> 24.0 [notice] To update, run: python3.9 -m pip install --upgrade pip

However, the very first thing the actions do is upgrade pip, so not sure why this message shows up. Hopefully it also disappears with setup-python@v5

@hugobuddel
Copy link
Contributor Author

I misread what github actually says. It complained that actions/setup-python@v4 and codecov/codecov-action@v3 were using node v16, not that we should upgrade to those versions. So I also upgraded codecov-action to v4, in the hope that it does not break anything.

The main requirement of codecov-action v4 seems that it now requires a token to upload coverage. I believe we already use that, and otherwise we should.

@hugobuddel hugobuddel marked this pull request as draft May 13, 2024 07:41
@hugobuddel
Copy link
Contributor Author

Oh, codecov-action@v4 does not work:

Error: Codecov token not found. Please provide Codecov token with -t flag.
Warning: Codecov: Failed to properly create commit: The process '/home/runner/work/_actions/codecov/codecov-action/v4/dist/codecov' failed with exit code 1

So we should provide a token somehow. Putting this to Draft.

And perhaps the action should not be okay if it doesn't actually succeed? Or maybe that is fine since it is not critical?

@teutoburg
Copy link
Contributor

So we should provide a token somehow.

We are using tokens for codecov on some projects, but not on all AFAIK. We probably should just do that for all of them...

Copy link

codecov bot commented May 13, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (e7622bb) to head (604931c).

Current head 604931c differs from pull request most recent head fd239bf

Please upload reports for the commit fd239bf to get more accurate results.

Additional details and impacted files
@@            Coverage Diff            @@
##            master       #22   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            2         2           
  Lines            7         7           
=========================================
  Hits             7         7           

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

@teutoburg
Copy link
Contributor

Also, perhaps we should think about how to deal with the poetry branch here. I think half of our projects already use that "version" of the DevOps, but we're not applying this fix to master. Perhaps we should soon-ish merge poetry into master, and perhaps rename it to main at the same time, since we're already breaking things anyway most likely.

To deal with projects that don't (and won't) use poetry: I already did some experimentation using GitHub's rather new "custom repo properties", which can be accessed from GH actions (although I never took it that far). Maybe the simplest solution would be to add a "poetry=True" property on the repos where it applies (I already did that some time ago, but need to check if it's up to date), and then let the DevOps check for that property. Ideally such that if no such property is found at all, just default to non-poetry...

Copy link
Contributor

@teutoburg teutoburg left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this! I've seen that Node16 warning for a while, but never bothered to care 🙃

@teutoburg
Copy link
Contributor

Would this work:

token: ${{ secrets.CODECOV_TOKEN }}

@hugobuddel hugobuddel marked this pull request as ready for review May 13, 2024 08:06
@hugobuddel
Copy link
Contributor Author

It does work with env as well. I added a organization-wide secret. That would be fine right?

@hugobuddel
Copy link
Contributor Author

Also, perhaps we should think about how to deal with the poetry branch here. I think half of our projects already use that "version" of the DevOps, but we're not applying this fix to master. Perhaps we should soon-ish merge poetry into master, and perhaps rename it to main at the same time, since we're already breaking things anyway most likely.

To deal with projects that don't (and won't) use poetry: I already did some experimentation using GitHub's rather new "custom repo properties", which can be accessed from GH actions (although I never took it that far). Maybe the simplest solution would be to add a "poetry=True" property on the repos where it applies (I already did that some time ago, but need to check if it's up to date), and then let the DevOps check for that property. Ideally such that if no such property is found at all, just default to non-poetry...

Oh I didn't think about that; I'll let you decide. We can also merge this into both master and poetry for now?

@teutoburg
Copy link
Contributor

That would be fine right?

I see now a warning in e.g. skycalc_ipy (which had a token before) that the secret is overridden by a org-wide secret. Let's see if this breaks anything, but if the org-wide option work, I'd prefer that, because then we don't need to think about it everywhere...

@teutoburg
Copy link
Contributor

We can also merge this into both master and poetry for now?

Probably the best option in the short run. I'm a bit too busy now to deal with the long-term option I described above, but want to do it eventually...

- actions/setup-python@v5 updates node version from node16 to node20
- actions/checkout@v4 seems to be the standard
- actions/setup-node@v4 recommends specifying a node version
- codecov-action to v4

This change is prompted by github complaining that
"Node.js 16 actions are deprecated. Please update the following actions
to use Node.js 20: actions/setup-python@v4, codecov/codecov-action@v3."

The new codecov action requires the use of CODECOV_TOKEN
@hugobuddel
Copy link
Contributor Author

Squashed so it can be easily cherrypicked for the poetry branch

@hugobuddel hugobuddel merged commit 96b50a7 into master May 23, 2024
12 checks passed
@hugobuddel hugobuddel deleted the hb/updategithubactions branch May 23, 2024 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants