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

refactor: reads supported Foundry version from an environment variable #11046

Conversation

arthurgousset
Copy link
Member

@arthurgousset arthurgousset commented Jun 19, 2024

Description

  1. reads an environment variable (SUPPORTED_FOUNDRY_VERSION) defined at the celo-org (GitHub org) level.
  2. updates the Foundry installation version to use the defined variable for consistency across workflows in celo-org/celo-monorepo and celo-org/developer-tooling.

The overall objective is to update Foundry versions in lock-steps. See more context on pros and cons in:

Other changes

None.

Tested

Tested on CI. This only changes CI workflows.

Related issues

Backwards compatibility

Yes.

Documentation

Yes in code comments and this PR description.

@arthurgousset arthurgousset changed the title refactor: reads supported Foundry version from an environment variable #271 refactor: reads supported Foundry version from an environment variable Jun 19, 2024
@arthurgousset
Copy link
Member Author

arthurgousset commented Jun 19, 2024

Expect the workflows to fail, because the SUPPORTED_FOUNDRY_VERSION is not yet defined at the celo-org (GitHub org) level. Context #11032 (comment).

@arthurgousset
Copy link
Member Author

Expect the workflows to pass, because the SUPPORTED_FOUNDRY_VERSION is defined at the celo-org (GitHub org) level. See Slack confirmation.

@arthurgousset arthurgousset marked this pull request as ready for review June 19, 2024 15:09
@arthurgousset arthurgousset requested a review from a team as a code owner June 19, 2024 15:09
@arthurgousset
Copy link
Member Author

Waiting to confirm whether I should merge this branch against master or release/core-contracts/12 .

@arthurgousset arthurgousset requested a review from a team June 19, 2024 15:15
@arthurgousset arthurgousset changed the base branch from master to release/core-contracts/12 June 19, 2024 15:31
@arthurgousset arthurgousset marked this pull request as draft June 19, 2024 15:34
@arthurgousset
Copy link
Member Author

arthurgousset commented Jun 19, 2024

Converting this into a draft PR again. I'll merge this branch into release/core-contracts/12, but I need to cherry-pick my commits, and cut a new branch from release/core-contracts/12 that doesn't include the latest mento commits against master.

@arthurgousset arthurgousset force-pushed the arthurgousset/refactor/foundry-version-env-variable branch from cc9922c to 8f039a2 Compare June 19, 2024 15:56
@arthurgousset
Copy link
Member Author

That should be done with 8f039a2. Waiting for CI to pass before marking this as ready for review.

@arthurgousset arthurgousset marked this pull request as ready for review June 21, 2024 10:14
@arthurgousset
Copy link
Member Author

CI didn't complete, because runner didn't pick the "Build & Integration Tests / Install dependencies" workflow.
Cancelled, and re-running failed jobs.

@arthurgousset
Copy link
Member Author

All workflows are green, but waiting for "Protocol Compatibility" to report. Not sure why this is stuck at the moment.

image

@arthurgousset
Copy link
Member Author

When I run the "Protocol Compatibility" command locally, the command passes:

$ yarn
$ yarn build
$ yarn --cwd packages/protocol test compatibility/

# ...
  76 passing (4s)
  2 pending

✨  Done in 196.13s.

Context:

- name: Protocol Compatibility
command: |
yarn --cwd packages/protocol test compatibility/

@arthurgousset arthurgousset force-pushed the arthurgousset/refactor/foundry-version-env-variable branch from 71afc79 to e886bb1 Compare July 2, 2024 16:42
@arthurgousset arthurgousset force-pushed the arthurgousset/refactor/foundry-version-env-variable branch from e886bb1 to fb3b71d Compare July 2, 2024 16:59
Copy link

refactor: reads supported Foundry version from an environment variable

Generated at commit: e886bb107683aa146135035501c76583dbf748e3

🚨 Report Summary

Severity Level Results
Contracts Critical
High
Medium
Low
Note
Total
2
3
0
15
41
61
Dependencies Critical
High
Medium
Low
Note
Total
0
0
0
0
0
0

For more details view the full report in OpenZeppelin Code Inspector

@arthurgousset
Copy link
Member Author

arthurgousset commented Jul 2, 2024

The problem with the missing (or "expected") workflow was that the condition that is evaluated before running the steps could never evaluate to true here, because it was running against release/core-contracts/** branches.

Now that the condition includes release branches, this step can run:

if: |
github.base_ref == 'master' || contains(github.base_ref, 'release') || contains(github.base_ref, 'production') ||
contains(needs.install-dependencies.outputs.all_modified_files, 'packages/protocol') ||
contains(needs.install-dependencies.outputs.all_modified_files, ',package.json') ||
contains(needs.install-dependencies.outputs.all_modified_files, ',yarn.lock') ||

The required changes was:

- github.base_ref == 'master' || contains(github.base_ref, 'production') ||
+ github.base_ref == 'master' || contains(github.base_ref, 'release') || contains(github.base_ref, 'production') ||

This was fixed in #11080, which was merged in the meantime.

The workflows are all green now ✅

Copy link
Contributor

@soloseng soloseng left a comment

Choose a reason for hiding this comment

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

🚀

@arthurgousset arthurgousset merged commit 7dfd467 into release/core-contracts/12 Jul 3, 2024
21 checks passed
@arthurgousset arthurgousset deleted the arthurgousset/refactor/foundry-version-env-variable branch July 3, 2024 10:18
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.

Fix Foundry version across GitHub workflows
3 participants