Skip to content

Gitlab Client: Parse owner and repository in a url safe way #698

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
Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Gitlab Client: Parse owner and repository in a url safe way #698

wants to merge 1 commit into from

Conversation

@StingRayZA
Copy link

Instead of stitching the 'owner' and 'repository' together with a %2F - rather join them with a / and let python make them conformant.
This way, if we have any / characters embedded in either value, they too will be correctly translated

@sduenas
Copy link
Member

sduenas commented Oct 23, 2020

Thanks for the PR. I need some time to review it. I'll do it this weekend.

@StingRayZA
Copy link
Author

Hi 👋
Could I request you label this PR with hacktoberfest-accepted ?

@sduenas
Copy link
Member

sduenas commented Oct 30, 2020

@StingRayZA I cannot merge the PR right now because it will break other parts of the tool chain. They expect the URLs as they are now. Would you mind to rebase your PR to master and improve the commit message. It is described in the contributing rules.

I won't merge it but I will set it as a hacktober contribution.

@StingRayZA
Copy link
Author

Sure, no problem I'll rebase and reformat the commit message.

@StingRayZA
Parse owner and repository in a url safe way
5fa96f1 
This allows us to retain the '/' separation of URL pieces in any gitlab
urls that have been stored in the object. This change also lays the
groundwork for a fix in the -elk repository to allow multiple nested
gitlab group layers in gitlab URLs.

See also: chaoss/grimoirelab-elk#946

Signed-off-by: Raimund Hook <raimund.hook@exfo.com>
@StingRayZA
Copy link
Author

@sduenas Rebased on master with a reformatted message.
It also links to chaoss/grimoirelab-elk#946 which is where I started thinking about this issue. I think my hack in that issue is a bit naive, but together with this one - my local copy of grimoirelab is working well now.

sduenas
sduenas approved these changes Nov 3, 2020
View reviewed changes
Copy link
Member

@sduenas sduenas left a comment

Choose a reason for hiding this comment

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

LGTM

As I said, I'll merge it when we fix the related problems in ELK.

@sduenas sduenas added the hacktoberfest-accepted Hacktoberfest tasks 2020 label Nov 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sduenas sduenas sduenas approved these changes

Assignees

No one assigned

Labels

hacktoberfest-accepted Hacktoberfest tasks 2020

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@StingRayZA @sduenas