Skip to content

DP Panel: Drop protocol check when creating a new item#2028

Merged
norascheuch merged 3 commits intomainfrom
nora/polish-url-helper
Feb 1, 2023
Merged

DP Panel: Drop protocol check when creating a new item#2028
norascheuch merged 3 commits intomainfrom
nora/polish-url-helper

Conversation

@norascheuch
Copy link
Copy Markdown
Contributor

Checklist

  • CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
  • Issues have been created for any UI or other user-facing changes made by this pull request.
  • [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

@norascheuch norascheuch requested a review from a team as a code owner January 31, 2023 14:46
Copy link
Copy Markdown
Contributor

@shati-patel shati-patel left a comment

Choose a reason for hiding this comment

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

Looks good! 🎉 Would you mind adding a short PR description?


it("should handle valid urls", () => {
expect(getNwoFromGitHubUrl("https://github.com/foo/bar")).toBe("foo/bar");
expect(getNwoFromGitHubUrl("http://github.com/foo/bar")).toBe("foo/bar");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we also add a check for just "github.com/foo/bar"?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

good idea!

@norascheuch
Copy link
Copy Markdown
Contributor Author

norascheuch commented Feb 1, 2023

I think the whole requirement is questionable but it should work now.

@norascheuch norascheuch merged commit 9f240c8 into main Feb 1, 2023
@norascheuch norascheuch deleted the nora/polish-url-helper branch February 1, 2023 19:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants