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

Switch to plaintext fingerprint #98

Merged
merged 3 commits into from
Oct 10, 2022

Conversation

weineran
Copy link
Contributor

@weineran weineran commented Oct 7, 2022

* Changed repo ("machine") fingerprint to plaintext full_name of repo
* This also allowed me to remove a lot of the code that was specific to
the schedule event.
* It looks like repository.html_url and repository.full_name are already
included in the schedule event:
https://github.com/dhimmel/dump-actions-context/actions/runs/3191708047/jobs/5208342350
(but we should test)
@weineran weineran added the enhancement New feature or request label Oct 7, 2022
@weineran weineran requested a review from zricethezav October 7, 2022 04:07
@weineran weineran self-assigned this Oct 7, 2022
@weineran
Copy link
Contributor Author

weineran commented Oct 7, 2022

@zricethezav Can you review? But do not merge yet because:

  • package-lock.json changed a lot, which it shouldn't. Need to check that.
  • Need to test, especially "schedule" event
  • We should clear out the machines list before we merge, then we should merge and release, then clear out the machines list again. Then monitor for a couple days to see if anyone is pinned to a minor version.

@zricethezav
Copy link
Collaborator

@weineran changes look good to me. I'm glad we can remove the "schedule" event backfill code. Let me know if you want me to build dist then rebase to see if that fixes the package-lock.json diff.

Gonna be busy for a few hours this morning, but will have some time this afternoon

Andrew Weiner added 2 commits October 8, 2022 06:03
* Reinstalled node modules with correct node version to hopefully fix package-lock.json
@weineran
Copy link
Contributor Author

weineran commented Oct 8, 2022

Let's plan to merge this on Monday during our meeting. We should clear out the machines list right before we merge, then we should merge and release, then clear out the machines list again. Then monitor for a couple days to see if anyone is pinned to a minor version.

@zricethezav zricethezav merged commit 1938557 into master Oct 10, 2022
@weineran
Copy link
Contributor Author

weineran commented Oct 11, 2022 via email

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

Successfully merging this pull request may close these issues.

2 participants