Skip to content

Fix for the new GitHub token match#104

Merged
mattyjones merged 1 commit intoN0MoreSecr3ts:masterfrom
Shashank-In:patch-1
Apr 29, 2021
Merged

Fix for the new GitHub token match#104
mattyjones merged 1 commit intoN0MoreSecr3ts:masterfrom
Shashank-In:patch-1

Conversation

@Shashank-In
Copy link
Copy Markdown
Contributor

Github token now starts with ghp_

Github token now starts with `ghp_`
@qlty-cloud-legacy
Copy link
Copy Markdown

Code Climate has analyzed commit 8ffee87 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Copy Markdown
Collaborator

@mattyjones mattyjones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Shashank-In has this been tested with both versions of the keys?

@mattyjones
Copy link
Copy Markdown
Collaborator

@Shashank-In Thanks for this, I was unaware of the change! One thing, not a concern at this point but please pull from develop when submitting changes. I try my best to keep the master branch as stable as possible and not accept PR's against it unless unit and integration tests are also provided.

In this case, the change is near trivial in terms of code modification and I have automated testing to cover this but for larger code changes this would be rejected, unless 100% code coverage was also provided until it can get pulled against develop as that is always current, the master is typically only for proven code.

@Shashank-In
Copy link
Copy Markdown
Contributor Author

Hi @mattyjones
Yes, the regex will work for both cases' old one, and the new one as the number of characters is still 40, just that a _ has been introduced.
Sorry I didn't read the doc. Next time I will make any PR on the dev.
I have already tested it locally. So this will work.

@Shashank-In
Copy link
Copy Markdown
Contributor Author

Hi @mattyjones
Can we merge this?

@mattyjones
Copy link
Copy Markdown
Collaborator

@Shashank-In I will merge this week. I have a bunch of stuff to update and will be cutting a new release, to include this, within the next few days.

@mattyjones mattyjones merged commit ecd2b95 into N0MoreSecr3ts:master Apr 29, 2021
@9oelM
Copy link
Copy Markdown

9oelM commented Sep 24, 2021

Could we have another release please?

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.

3 participants