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

Propose a Github Workflow for release process #119

Merged

Conversation

snowiow
Copy link
Contributor

@snowiow snowiow commented Jan 30, 2023

Issue #, if available:

Description of changes:
This is a proposal for an automated release process. We do it the same way in this repo. We thought it's a bit error prone to manually run ncc with the actual changes one has done, instead of having this as an automated workflow. Also the diff gets really big by committing the dist/index.js changes, which can't reliably be reviewed, opening a potential door for vulnerabilities.
The exact publish process via this action is documented in the README as part of this PR.

Alternatively you could go with a similar approach like the amazon-ecr-login.

If you go with one of the proposed solutions, this PR shouldn't be merged anymore. The manual work in that PR would be taken over by the publish workflow.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Check any applicable:

  • Were any files moved? Moving files changes their URL, which breaks all hyperlinks to the files.

@taoyong-ty
Copy link
Contributor

would it be possible to create a release in your forked repo, so that I can better understand what exactly the new action would do?

@snowiow
Copy link
Contributor Author

snowiow commented Jan 31, 2023

I'm not sure if it doesn't work because it's fork, but I get this error here: https://github.com/snowiow/aws-codebuild-run-build/actions/runs/4052211284/jobs/6971372794

On the Github Action we maintain, we use the exact same workflow. You can have a look at this run for reference: https://github.com/moia-oss/aws-codepipeline-trigger/actions/runs/3988342689/jobs/6839365693

@taoyong-ty
Copy link
Contributor

@snowiow I tried the "amazon-ecr-login" approach, but the "commit" step failed by the branch protection policy. https://github.com/aws-actions/aws-codebuild-run-build/actions/runs/4059164834/jobs/6986940329 I am afraid the publish workflow would also fail for the same reason.

Did you have to adjust any branch protection setting for your action?

@snowiow
Copy link
Contributor Author

snowiow commented Feb 3, 2023

Hi, we have branch protection set for the codepipeline-trigger repository and the action was still able to create the tags. About what setting are you talking exactly?

@taoyong-ty
Copy link
Contributor

taoyong-ty commented Feb 3, 2023

For example, this is a failed run https://github.com/aws-actions/aws-codebuild-run-build/actions/runs/4059164834/jobs/6986940329

remote: error: GH006: Protected branch update failed for refs/heads/main.        
remote: error: Changes must be made through a pull request. Required status check "ci-unit-tests" is expected.

There are two errors:

  1. "Required status check "ci-unit-tests" is expected." - I think I can fix thie one by updating the trigger (https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#onworkflow_runbranchesbranches-ignore)
  2. "Changes must be made through a pull request." - this one is what I am concerned about, as I am not sure if it is a good idea to lift this requirement for the sake of automating the packaging/release.

And here is the branch protection setting of this repo
Screen Shot 2023-02-03 at 12 58 59 PM

@snowiow
Copy link
Contributor Author

snowiow commented Feb 7, 2023

Yes, I think the ecr login approach doesn't work with those settings, because it pushes to the main branch directly. So probably they don't have branch protection enabled, but I'm not sure about their settings.

In regards to the action we use, it doesn't push to the main branch directly, but creates a separate tree and commits it there as I understand it. See here. We have the same protection rule and that action works for us nonetheless.

@taoyong-ty
Copy link
Contributor

sounds good. Let's merge this pr and see how it works.

@taoyong-ty taoyong-ty merged commit ff9a3b7 into aws-actions:main Feb 8, 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.

2 participants