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

Action allowing us to publish backported changes #130

Merged
merged 9 commits into from
Dec 12, 2023

Conversation

witmicko
Copy link
Contributor

@witmicko witmicko commented Nov 29, 2023

We're using hardcoded build source references and target versions to ensure we have clear control over what gets deployed where. This action is tailored to only create pull requests against the gh-pages branch.

example action run:
https://github.com/witmicko/phishing-warning/actions/runs/7033539537
it builds and commits changes to gh-pages-backport branch, I then manually created this PR
witmicko#1

uses: actions/checkout@v3
with:
ref: 'gh-pages'
path: 'gh-pages'
Copy link
Member

Choose a reason for hiding this comment

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

Rather than checking out this branch as a sub path, have you considered using the same github-actions deploy action we use elsewhere?

uses: peaceiris/actions-gh-pages@068dc23d9710f1ba62e86896f84735d869951305

We can use a matrix to iterate over each version target as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

tried this first but actions were failing when it ran them, will revisit this later tough as it is more elegant solution

@Gudahtt
Copy link
Member

Gudahtt commented Nov 29, 2023

Is this meant as an alternative to #81 ?

Grouping by major did seem like a nicer way of handling this going forward

@witmicko
Copy link
Contributor Author

Is this meant as an alternative to #81 ?

Grouping by major did seem like a nicer way of handling this going forward

no, not alternative, just stop-gap for this backport, Im also in favour of #81

@witmicko witmicko requested a review from Gudahtt December 4, 2023 16:08
Copy link
Contributor

@NicholasEllul NicholasEllul left a comment

Choose a reason for hiding this comment

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

Looks good as a temporary backport action 👍 Noting for context that this is here to facilitate backporting bug fixes until something like #81 can be incorporated.

Update: i see this was already noted above

Comment on lines 6 to 8
env:
BASE_REF: v2.0.1-from-tag
VERSIONS: v1.0.0,v1.1.0,v1.2.1,v1.2.2,v2.0.0,v2.0.1,v2.1.0
Copy link
Contributor

Choose a reason for hiding this comment

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

Did we hardcode this to prevent others from using it incorrectly and requiring a review for each change?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, also not using inputs as they can have mistakes and are harder to check

@NicholasEllul NicholasEllul self-requested a review December 4, 2023 16:26
NicholasEllul
NicholasEllul previously approved these changes Dec 4, 2023
Copy link
Member

@Gudahtt Gudahtt left a comment

Choose a reason for hiding this comment

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

LGTM!

@witmicko witmicko merged commit 7fd6b77 into MetaMask:main Dec 12, 2023
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.

3 participants