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

Update README.rst #2201

Closed
wants to merge 25 commits into from
Closed

Conversation

alexanderrichards
Copy link
Contributor

This is a commit just to create a new PR and we have no intention of merging this.

This is a commit just to create a new PR and we have no intention of merging this.
@alexanderrichards alexanderrichards temporarily deployed to Integrated Pull Request September 12, 2023 10:17 — with GitHub Actions Inactive
@alexanderrichards
Copy link
Contributor Author

closing this PR as it correctly asked me to approval before continuing

@alexanderrichards
Copy link
Contributor Author

re-opening to further discussion. Do we want the linting to run before we have to approve the rest or should we keep it as is where approval is needed for everything?

@alexanderrichards
Copy link
Contributor Author

in fact we could only have approval for those bits that need access to secrets?

@egede
Copy link
Member

egede commented Sep 12, 2023

The best would be to have it only for the parts needing secrets. In that way an outsider can discover most problems without our involvement.

I am a bit confused about how it actually will work. I thought it would not ask for approval if you were a developer within the team, but the comments above seems to indicate that it asked you to approve your own PR?

@alexanderrichards
Copy link
Contributor Author

All PRs run the same github action "CI Pust Testing". To allow the jobs within to access the secrets we use the on: pull_request_target which will by default run only the code from the base which is obviously safe. We then modify the checkout command to checkout the head of the PR and hence the potentially unsafe code. In order to police the running of this code we say that it has to run the tests in the environment Integrate Pull Request which we protect from running until approval is given by either myself, you or mark.

In practise this means that before the tests (that define to run in the environment) will run we get an email and have to click an approve button.

@alexanderrichards
Copy link
Contributor Author

I thought it would not ask for approval if you were a developer within the team,

I have checked the box that says "allow administrators to bypass protection rules". I guess this means that it will still wait for approval but that we should be able to click a button somewhere to allow it to proceed without. At that point though it's easy enough to just click approve since unlike with the code reviews we can approve ourselves

@egede
Copy link
Member

egede commented Oct 23, 2023

I believe this can just be closed. Reopen if required.

@egede egede closed this Oct 23, 2023
@alexanderrichards alexanderrichards deleted the alexanderrichards-patch-1 branch November 27, 2023 11:48
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