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

chore: switch from github/checkout @v2 to @V3 #30069

Closed
wants to merge 1 commit into from

Conversation

ghassanmas
Copy link
Member

@ghassanmas ghassanmas commented Mar 15, 2022

Since some tests fails possobly because v2 isn't supported or
maintained as v3.)

There seems to be an issue with github action v2, I have reported an issue on the action repo here But I think is better to use v3. If the tests passes now it means that has resolved it. Also the action repo itself start to use v3 for their workflow, hence

As it turns out and repoted below the test failing weren't related to checkout:
version rather something hast

Well to be fair, it turns out it wasn't related because v2/v3, rather it was some temp issue with microsoft packages hence actions/virtual-environments#5236 . That being said I am not sure if we I shall close this or keep but rename the commit message to reflect what is suppose to mean

Supporting information

Testing instructions

Just make sure to run this PR or its effect with self hosted runner (if there are?).
Since github/checkout@v3 only supprot runner :

Updated to the node16 runtime by default
This requires a minimum Actions Runner version of v2.285.0 to run, which is by default available in GHES 3.4 or later.
ref

If there are no self hosted runner then you can discard it

@openedx-webhooks openedx-webhooks added needs triage open-source-contribution PR author is not from Axim or 2U labels Mar 15, 2022
@openedx-webhooks
Copy link

Thanks for the pull request, @ghassanmas! I've created OSPR-6523 to keep track of it in JIRA, where we prioritize reviews. Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

  • supporting documentation
  • Open edX discussion forum threads
  • timeline information ("this must be merged by XX date", and why that is)
  • partner information ("this is a course on edx.org")
  • any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

@ghassanmas ghassanmas changed the title Fix failing tests due github action Fix: failing tests due github action Mar 15, 2022
@ghassanmas
Copy link
Member Author

ghassanmas commented Mar 15, 2022

Well to be fair, it turns out it wasn't related because v2/v3, rather it was some temp issue with microsoft packages hence actions/runner-images#5236 .
That being said I am not sure if we I shall close this or keep but rename the commit message to reflect what is suppose to mean

  Github checkout v3 is using node-16, so its expected to be more
  secure than v2.
  More info:
  https://github.com/actions/checkout/releases/tag/v3.0.0
@ghassanmas ghassanmas changed the title Fix: failing tests due github action chore: switch from github/checkout @v2 to @V3 Mar 16, 2022
@ghassanmas
Copy link
Member Author

This ready for review

@aht007
Copy link
Member

aht007 commented Apr 4, 2022

There was an issue with repo metadata generation in Microsoft packages and hence attempts to fetch the metadata would cause checksum issues. That being said the issue was resolved the same day. We are currently using checkout@v2 across openedx and just changing it here on platform would make it inconsistent and that too without any issues faced due to it. But I would like to have @awais786's views on it.

@UsamaSadiq
Copy link
Member

Since the target issue for this change is not the concern anymore and we want to keep the behavior consistent across all the edx/openedx workflows so this change is unnecessary for now. We can close this for now and introduce this change later on if we face any issues due to checkout-v2 in future.

@ghassanmas thanks for your efforts on this issue but we'll have to close this for now.

@openedx-webhooks
Copy link

@ghassanmas Even though your pull request wasn’t merged, please take a moment to answer a two question survey so we can improve your experience in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
open-source-contribution PR author is not from Axim or 2U rejected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants