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.

Sign up for GitHub

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

Jump to bottom

Support for using URL-encoded paths as group or project ids #550

Merged
merged 5 commits into from
Nov 3, 2023
Merged

Support for using URL-encoded paths as group or project ids #550

merged 5 commits into from
Nov 3, 2023

Conversation

PMExtra
Copy link
Contributor

@PMExtra PMExtra commented Oct 25, 2023

@PMExtra PMExtra requested a review from a team as a code owner October 25, 2023 02:35
@PMExtra PMExtra requested review from Toa741 and removed request for a team October 25, 2023 02:35
@PMExtra PMExtra marked this pull request as draft October 25, 2023 15:15
@PMExtra PMExtra marked this pull request as ready for review October 27, 2023 08:15
@PMExtra
Copy link
Contributor Author

PMExtra commented Oct 27, 2023

I changed the existing constructor to only support the new ProjectId or GroupId if the class is internal. Else, I marked the old constructor be obsolete and made a new one.

Maybe we should unify the client implementations classes to be internal instead of public? But it's a breaking change, although most users will never create a client by manually calling the constructor.

@PMExtra PMExtra requested review from meziantou and Toa741 October 30, 2023 02:51
@PMExtra PMExtra requested a review from Toa741 November 1, 2023 03:23
@meziantou meziantou removed their request for review November 2, 2023 18:37
@PMExtra
Copy link
Contributor Author

PMExtra commented Nov 3, 2023

@Toa741 I think it's ready to merge. Please let me know if there is anything else.

@Toa741
Copy link
Contributor

Toa741 commented Nov 3, 2023

@Toa741 I think it's ready to merge. Please let me know if there is anything else.

Alright :), will make a final review

@Toa741 Toa741 merged commit 44157b8 into ubisoft:main Nov 3, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Toa741 Toa741 Toa741 left review comments

@meziantou meziantou meziantou left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@PMExtra @Toa741 @meziantou