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

Skip pull request #13

Open
Eomm opened this issue Nov 21, 2021 · 11 comments
Open

Skip pull request #13

Eomm opened this issue Nov 21, 2021 · 11 comments
Labels
enhancement New feature or request

Comments

@Eomm
Copy link
Contributor

Eomm commented Nov 21, 2021

Right now the process to release a module would be:

  • trigger the action manually
  • go to the PRs page and merge the new PR
  • approve the optic otp

I wonder if we can skip the 2nd step and go straightforward from triggering the action and approve the optic OTP.

In this way:

  • if the publish fails the main branch does not contain the wrong version
  • the flow is much more simple for the user. Action: trigger the workflow, Reaction: optic notification
  • the new version should be committed only if the npm release is successfully
@simoneb
Copy link
Member

simoneb commented Nov 21, 2021

What you describe was the original idea and at some point we had that implementation and we tried it out, just to realize that it wouldn't work because often the main/master branch is protected, meaning that direct pushes won't work.

Hence, we opted for this alternative solution which opens a PR. Any contributor can approve and merge the PR, thereby circumventing the issue.

I have already reached out to @salmanm about removing the need to have a backing server app since this recent change in github: https://github.blog/changelog/2021-10-06-github-actions-workflows-triggered-by-dependabot-prs-will-respect-permissions-key-in-workflows/

I am not sure but I don't think that even with that we'll be able to skip the PR. In any case, I don't have big issues with this action opening a PR. The PR will also contain a preview of what will be released so a maintainer can review what's going to happen before making it happen.

@Eomm
Copy link
Contributor Author

Eomm commented Nov 21, 2021

I think it could work instead.

Reading the permission docs we need:

I agree that the commit list is useful, but providing the semver at the GH Action run assumes that the developer has already the commit list

@simoneb
Copy link
Member

simoneb commented Nov 21, 2021

I think it could work instead.

Why? If a branch is protected you can't push to it, and I assume this is true regardless of how you try to push to it (though we haven't tried I believe)

@Eomm
Copy link
Contributor Author

Eomm commented Nov 21, 2021

Oh, I get it now. You were meaning these GH protection rules

image

I was thinking about the default main protection rules, not the additional one TBH.

So do you agree that:

  • wide organizations need the PR process to support more granular permissions and rules
  • a single maintainer could apply for the light process to release the modules

?

@simoneb
Copy link
Member

simoneb commented Nov 21, 2021

Yes, I agree with your conclusions.

In the current phase we are aiming to satisfy the scenario with most constraints. Once we have that working, we can then enable features which enable simpler scenarios.

@simoneb simoneb added the enhancement New feature or request label Dec 7, 2021
@simoneb
Copy link
Member

simoneb commented Dec 7, 2021

this is on hold at the moment, we can't implement this feature

@Eomm
Copy link
Contributor Author

Eomm commented Jan 7, 2022

What is blocking?

@simoneb
Copy link
Member

simoneb commented Jan 7, 2022

This cannot work as discussed if the branch is protected

@simoneb
Copy link
Member

simoneb commented Jan 7, 2022

In any case, even though the initial idea for this action was to do everything automatically, without going through the PR, I now believe that having a PR is actually better than not having it. Why?

Because now this action can do several things, and having a PR which explains you what is going to happen once you merge it is a feature in my opinion.

I would suggest that we close this issue.

@Eomm
Copy link
Contributor Author

Eomm commented Jan 7, 2022

I would like to keep it open and work on this whenever I get a couple of hours.
I think that this feature fits best for (little) modules that ship one or two PRs.

@simoneb
Copy link
Member

simoneb commented May 20, 2022

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants