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

Increase identifier length from 20 to 44 #139

Merged
merged 2 commits into from
Feb 7, 2019

Conversation

djmitche
Copy link
Contributor

No description provided.

@djmitche djmitche self-assigned this Jan 28, 2019
@djmitche djmitche requested a review from a team January 28, 2019 21:11
Copy link
Contributor

@imbstack imbstack left a comment

Choose a reason for hiding this comment

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

This looks great to me!

Copy link

@walac walac left a comment

Choose a reason for hiding this comment

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

Oh, finally! God blesses you!

* `schedulerId`
* `organization`, `repository` (in tc-github)

* "slugids" -- up to 22 characters, always a slugid
Copy link
Contributor

Choose a reason for hiding this comment

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

slugids are always 22 characters right? for those identifiers which are slugids, we should validate that they're valid slugids, or else they'd be plain identifiers that just aren't 40 chars.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, you're right -- I'll change that to "exactly 22". It's already validated.

@djmitche
Copy link
Contributor Author

I'll leave this until early next week when @helfi92 and @owlishDeveloper are back.

Copy link
Contributor

@owlishDeveloper owlishDeveloper left a comment

Choose a reason for hiding this comment

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

awesome!

@djmitche
Copy link
Contributor Author

djmitche commented Feb 5, 2019

OK! I'll land this on Thursday, unless there are comments during the final-comment period.

@djmitche djmitche merged commit 2a2b1fb into taskcluster:master Feb 7, 2019
@djmitche
Copy link
Contributor Author

djmitche commented Feb 7, 2019

@OjaswinM noted that github's organization and repository strings are already allowed to be 100 characters. It's too bad we didn't spot that in the RFC process, but c'est la vie. If we had, I think we would not have decided to shrink those identifiers to 40 characters, so we'll leave them at 100.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants