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.

Sign up for GitHub

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

Jump to bottom

move genesis check to verify pr workflow #872

Merged
merged 1 commit into from
Jan 6, 2023
Merged

move genesis check to verify pr workflow #872

merged 1 commit into from
Jan 6, 2023

Conversation

demisx
Copy link
Collaborator

@demisx demisx commented Jan 4, 2023 •
edited
Loading

Goal

The goal of this PR is move Genesis check to the Verify PR workflow.

@demisx demisx self-assigned this Jan 4, 2023
@demisx demisx force-pushed the move-genesis-check-to-verify-pr branch 10 times, most recently from e58f835 to 269a1d6 Compare January 4, 2023 22:14
@codecov
Copy link

codecov bot commented Jan 4, 2023 •
edited
Loading

Codecov Report

Merging #872 (1aa519b) into main (b5d78e8) will not change coverage.
The diff coverage is n/a.

❗ Current head 1aa519b differs from pull request most recent head 4476057. Consider uploading reports for the commit 4476057 to get more accurate results

@@           Coverage Diff           @@
##             main     #872   +/-   ##
=======================================
  Coverage   84.54%   84.54%           
=======================================
  Files          17       17           
  Lines         809      809           
=======================================
  Hits          684      684           
  Misses        125      125

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@demisx demisx force-pushed the move-genesis-check-to-verify-pr branch 8 times, most recently from 9567d41 to ea31a44 Compare January 5, 2023 01:12
@demisx demisx marked this pull request as ready for review January 5, 2023 03:52
wilwade
wilwade reviewed Jan 5, 2023
View reviewed changes
Copy link
Collaborator

@wilwade wilwade left a comment

Choose a reason for hiding this comment

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

Few suggestions. I do wonder if there are some ways to simplify some of the structures. Be nice if the matrix and env vars could be shared somehow, but likely that abstraction would just be more confusing.

.github/workflows/verify-pr-commit.yml Outdated Show resolved Hide resolved
.github/workflows/verify-pr-commit.yml Outdated Show resolved Hide resolved
.github/workflows/verify-pr-commit.yml Outdated Show resolved Hide resolved
.github/workflows/verify-pr-commit.yml Outdated Show resolved Hide resolved
@demisx
Copy link
Collaborator Author

demisx commented Jan 5, 2023

Few suggestions. I do wonder if there are some ways to simplify some of the structures. Be nice if the matrix and env vars could be shared somehow, but likely that abstraction would just be more confusing.

We do have global .env section above .jobs to share global environment variables. The ones that are job specific, go into jobs.<job-name>.env one. I moved env.BIN_DIR to globals. Let me know if you see anything else that you think should be truly global/shared between the jobs.

demisx
demisx commented Jan 5, 2023
View reviewed changes
Copy link
Collaborator Author

@demisx demisx left a comment

Choose a reason for hiding this comment

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

I have processed @wilwade comments. Please re-review.

@demisx demisx requested a review from wilwade January 5, 2023 17:30
@demisx
Copy link
Collaborator Author

demisx commented Jan 5, 2023

The Frequency CI diagram has been updated with the new job as well:

Screenshot 2023-01-05 at 10 03 37 AM

wilwade
wilwade approved these changes Jan 5, 2023
View reviewed changes
Copy link
Collaborator

@wilwade wilwade left a comment

Choose a reason for hiding this comment

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

No other blocking changes from me

@demisx demisx force-pushed the move-genesis-check-to-verify-pr branch 3 times, most recently from 1f2b606 to 5f27c6b Compare January 5, 2023 20:25
wilwade
wilwade approved these changes Jan 5, 2023
View reviewed changes
Copy link
Collaborator

@wilwade wilwade left a comment

Choose a reason for hiding this comment

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

👍

saraswatpuneet
saraswatpuneet approved these changes Jan 5, 2023
View reviewed changes
Copy link
Collaborator

@saraswatpuneet saraswatpuneet left a comment

Choose a reason for hiding this comment

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

🙇

@demisx demisx force-pushed the move-genesis-check-to-verify-pr branch from 5f27c6b to 6f39cd0 Compare January 5, 2023 21:01
@demisx demisx force-pushed the move-genesis-check-to-verify-pr branch from 1aa519b to 4476057 Compare January 6, 2023 02:23
@demisx demisx merged commit f2ea5e7 into main Jan 6, 2023
@demisx demisx deleted the move-genesis-check-to-verify-pr branch January 6, 2023 02:25
abadaa pushed a commit that referenced this pull request Jan 10, 2023
@demisx @abadaa
move genesis check to verify pr workflow (#872)
71e63ae 
# Goal
The goal of this PR is move Genesis check to the Verify PR workflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wilwade wilwade wilwade approved these changes

@saraswatpuneet saraswatpuneet saraswatpuneet approved these changes

Assignees

@demisx demisx

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@demisx @wilwade @saraswatpuneet