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 unscoped routes in location targets on gitlab #12774

Merged
merged 5 commits into from Aug 2, 2022
Merged

support unscoped routes in location targets on gitlab #12774

merged 5 commits into from Aug 2, 2022
+148 −212

Conversation

jamieklassen
Copy link
Member

Hey, I just made a Pull Request!

Resolves #12456 by removing the buildRawUrl function and making getProjectId and buildProjectUrl flexible enough to support both scoped and unscoped routes. I also refactored the tests a bit to give better feedback, and covered some missing edge cases.

I really tried to minimize the scope of the change to the app code, but the specific lines are not especially readable if you're not familiar with scoped and unscoped routes in GitLab. I can extract some helper functions with clearer names if desired.

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)
  • All your commits have a Signed-off-by line in the message. (more info)

@jamieklassen jamieklassen requested review from a team as code owners July 21, 2022 16:47
@github-actions
Copy link
Contributor

Changed Packages

Package Name Package Path Changeset Bump Current Version
@backstage/integration packages/integration patch v1.2.2

@jamieklassen jamieklassen changed the title Gitlab unscoped support unscoped routes in location targets on gitlab Jul 21, 2022
Jamie Klassen added 3 commits July 21, 2022 12:59
refactor tests to give clearer feedback
1de2669 
Instead of comments in the table test, use the features of jest that allow each
test to give a specific error message. This change also reduces the total number
of tests, but coverage should remain the same branch-wise.

Signed-off-by: Jamie Klassen <jklassen@vmware.com>
fake gitlab API more precisely
56a8ff1 
This allows this suite to detect when an unexpected path is passed to
the projects API.

Signed-off-by: Jamie Klassen <jklassen@vmware.com>
add coverage for "blob" folder edge case
3a22541 
Signed-off-by: Jamie Klassen <jklassen@vmware.com>
Jamie Klassen added 2 commits July 22, 2022 07:36
Look up project IDs for unscoped routes
1e1e436 
Signed-off-by: Jamie Klassen <jklassen@vmware.com>
Add changeset
1f27d83 
Signed-off-by: Jamie Klassen <jklassen@vmware.com>
@benjdlambert
Copy link
Member

I really want to get a few pairs of eyes on this, and tested by some other GitLab users too. Gonna ask about in Discord see if there's anyone that can help out. Is there any other GitLab users you know of @jamieklassen?

@jamieklassen
Copy link
Member Author

@OpenSourceZombie @mateiidavid @dehamzah @ItzEdgar @adrianke77 - you've all contributed to this part of the codebase in the past. Any chance you might have bandwidth to test this out?

@jamieklassen
Copy link
Member Author

@cregev also showed interest in the bug and might be able to help test?

@jamieklassen jamieklassen mentioned this pull request Jul 25, 2022
Closed
@github-actions
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity from the author. It will be closed if no further activity occurs. If you are the author and the PR has been closed, feel free to re-open the PR and continue the contribution!

@github-actions github-actions bot added the stale label Jul 29, 2022
@benjdlambert
Copy link
Member

Hey @jamieklassen - I'm pretty tempted just to merge this as is to be honest, and push it out for testing in the next release on Tuesday either way. Would be great if we could get someone to test it from the PR though before if that's possible, but if not, no biggie. Let's just use the next releases to test.

@github-actions github-actions bot removed the stale label Jul 29, 2022
benjdlambert
benjdlambert approved these changes Aug 2, 2022
View reviewed changes
Copy link
Member

@benjdlambert benjdlambert left a comment

Choose a reason for hiding this comment

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

Let's merge this, and see what happens and get some of the folks that use GitLab to try it once the release goes out today! 🎉

@benjdlambert benjdlambert merged commit eacf97c into backstage:master Aug 2, 2022
@ricardojlrufino ricardojlrufino mentioned this pull request Aug 5, 2022
Closed
@jamieklassen jamieklassen mentioned this pull request Mar 23, 2023
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@benjdlambert benjdlambert benjdlambert approved these changes

Assignees

@Rugvip Rugvip

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

static catalog locations don't support gitlab unscoped route targets
3 participants
@jamieklassen @benjdlambert @Rugvip