-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Bugfix/directory workflow #596
Bugfix/directory workflow #596
Conversation
My thinking is that to avoid the challenge of updating the DIRECTORY file across the forked repos, we could make it updated on merge to |
Thanks for your effort, I really appreciate that! |
P.S. see this comment. It seems that there may be an issue with the currently proposed approach. |
This pull request has been automatically marked as abandoned because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
I still haven't found a solution to update the I think a better route forward that avoids the PAT problems above, would be to configure an SSH deploy key for the repo, and then provide the private key as a secret for the workflows to deploy back to the GitHub master branch itself. Although this doesn't have the benefits of previewing the directory changes in the PR diff, at least every time a PR is merged the workflows can automatically commit the directory update Steps I would need help with from a repo admin:
@siriak does that make sense? If so would you be available to try that out? |
I will try doing this next weekend, I'm a little busy till then. Thank you! |
Basically what these deploy keys do is they allow pushing to master. I have just allowed pushing to it (in branch protection settings). This allows a little bit more than I need, but I feel like it's way simpler than maintaining these deploy keys in the future should anything change. Could you resolve the conflicts, and then I'll merge to see how it works? |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #596 +/- ##
=======================================
Coverage 94.51% 94.51%
=======================================
Files 277 277
Lines 22258 22258
=======================================
Hits 21037 21037
Misses 1221 1221 ☔ View full report in Codecov by Sentry. |
Just updated merge conflicts. Wanted to ask how you added the ssh key? Is it a secret for the repo (in which case I just need to add a couple lines to use something like the I had anticipated needing some lines like: - uses: webfactory/[email protected]
with:
ssh-private-key: ${{ secrets.DIRECTORY_DEPLOY_KEY }} In order to explicitly use the elevated privileges with the deploy key. |
I didn't add an ssh key. The workflow just uses some built-in authentication (I'm not terribly familiar with that) |
Let's see if DIRECTORY is updated after I merge this PR |
Pull Request Template
Description
This PR attempts to address the problem of the directory workflow not writing back the repo.
The full discussion is here: #591
Before merging, @siriak should try:
BOT_TOKEN
(classic is easiest, but if you dofine-grained
make sure to give itrepo
permission)master
branch protection, his own account is included in the Allow specific actors to bypass pull request requirements listType of change
Please delete options that are not relevant.
Checklist:
cargo clippy --all -- -D warnings
just before my last commit and fixed any issue that was found.cargo fmt
just before my last commit.cargo test
just before my last commit and all tests passed.mod.rs
file within its own folder, and in any parent folder(s).DIRECTORY.md
with the correct link.COUNTRIBUTING.md
and my code follows its guidelines.Please make sure that if there is a test that takes too long to run ( > 300ms), you
#[ignore]
that ortry to optimize your code or make the test easier to run. We have this rule because we have hundreds of
tests to run; If each one of them took 300ms, we would have to wait for a long time.