Skip to content

Rename checkver_token to gh_token and SCOOP_CHECKVER_TOKEN to SCOOP_GH_TOKEN#4832

Merged
rashil2000 merged 2 commits intodevelopfrom
unknown repository
Mar 21, 2022
Merged

Rename checkver_token to gh_token and SCOOP_CHECKVER_TOKEN to SCOOP_GH_TOKEN#4832
rashil2000 merged 2 commits intodevelopfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Mar 21, 2022

Checklist:

  • I have read the Contributing Guide.
  • I have updated the documentation accordingly.
  • I have updated the tests accordingly.

@rashil2000
Copy link
Copy Markdown
Member

rashil2000 commented Mar 21, 2022

Additionally, can you replace all instances of checkver_token with gh_token (4 in total) in this PR?

And add an entry in the changelog

@ghost ghost changed the title Properly filtering null input in dl function when private_hosts is not set in config Rename checkver_token to gh_token Mar 21, 2022
@ghost ghost changed the title Rename checkver_token to gh_token Rename checkver_token to gh_token and SCOOP_CHECKVER_TOKEN to SCOOP_GH_TOKEN Mar 21, 2022
@ghost
Copy link
Copy Markdown
Author

ghost commented Mar 21, 2022

@rashil2000 done, please review

@rashil2000
Copy link
Copy Markdown
Member

@rashil2000 rashil2000 merged commit ad3fc4f into ScoopInstaller:develop Mar 21, 2022
@ghost ghost deleted the private-repos-fix branch March 21, 2022 22:00
rashil2000 added a commit to ScoopInstaller/GithubActions that referenced this pull request Mar 21, 2022
@rashil2000
Copy link
Copy Markdown
Member

https://github.com/ScoopInstaller/GithubActions/blob/bd73d5711c4ad072c73e17d41b592e57d71892c6/src/Action/Scheduled.psm1#L24

This also needs to be updated then, I'll do it once this is merged

This can't be done yet. We will have to wait for a new release.

@niheaven
Copy link
Copy Markdown
Member

This tends to be a refactor, not a feature, will change the CHANGELOG next time.

@ghost
Copy link
Copy Markdown
Author

ghost commented Mar 22, 2022

This tends to be a refactor, not a feature, will change the CHANGELOG next time.

I was debating between these two and decided to go with feature since this change affects the users (env var change, configuration parameter change). However if refactoring in the sense you use it allows "breaking" changes, I agree this is refactoring.

@niheaven
Copy link
Copy Markdown
Member

niheaven commented Mar 22, 2022

Yes, adding a "breaking change" at the top of release notes for this one is a good idea, since it is UX change, right? 😄

@ghost
Copy link
Copy Markdown
Author

ghost commented Mar 22, 2022

Yes, adding a "breaking change" at the top of release notes for this one is a good idea, since it is UX change, right? 😄

Exactly. I searched in the changelog for something like this, but didn't find...

@niheaven
Copy link
Copy Markdown
Member

Exactly. I searched in the changelog for something like this, but didn't find...

Because we haven't had such BREAKING CHANGES since we have changelog...

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