Skip to content

Extend githubPermalinkRegex - #600

Merged
hanzei merged 5 commits into
mattermost:masterfrom
doc-sheet:patch-1
Feb 23, 2023
Merged

Extend githubPermalinkRegex#600
hanzei merged 5 commits into
mattermost:masterfrom
doc-sheet:patch-1

Conversation

@doc-sheet

@doc-sheet doc-sheet commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

Summary

Turns out . (dot) is a valid character for github repo name.
I couldn't find official info on what other characters are allowed there.

@doc-sheet
Extend githubPermalinkRegex
6884763 
Turns out `.` (dot) is a valid character for github repo name.
I couldn't find official info on what other characters are allowed there.
@doc-sheet
doc-sheet requested a review from hanzei as a code owner November 3, 2022 06:54
@mattermod

mattermod commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

Hello @doc-sheet,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

Per the Mattermost Contribution Guide, we need to add you to the list of approved contributors for the Mattermost project.

Please help complete the Mattermost contribution license agreement?
Once you have signed the CLA, please comment with /check-cla and confirm that the CLA check is green.

This is a standard procedure for many open source projects.

Please let us know if you have any questions.

We are very happy to have you join our growing community! If you're not yet a member, please consider joining our Contributors community channel to meet other contributors and discuss new opportunities with the core team.

@codecov-commenter

codecov-commenter commented Nov 3, 2022 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Base: 15.63% // Head: 15.67% // Increases project coverage by +0.03% 🎉

Coverage data is based on head (348a044) compared to base (9d6cb4b).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 348a044 differs from pull request most recent head 44d4fa0. Consider uploading reports for the commit 44d4fa0 to get more accurate results

Additional details and impacted files 
@@            Coverage Diff             @@
##           master     #600      +/-   ##
==========================================
+ Coverage   15.63%   15.67%   +0.03%     
==========================================
  Files          15       15              
  Lines        5243     5231      -12     
==========================================
  Hits          820      820              
+ Misses       4380     4368      -12     
  Partials       43       43
Impacted Files Coverage Δ
server/plugin/plugin.go 3.69% <100.00%> (+<0.01%) ⬆️
server/plugin/cluster.go 0.00% <0.00%> (ø)
server/plugin/command.go 8.32% <0.00%> (+0.16%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

hanzei
hanzei approved these changes Nov 3, 2022
View reviewed changes

@hanzei hanzei left a comment

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.

Good catch, thanks for the fix 👍

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Nov 3, 2022
@hanzei
hanzei requested a review from javaguirre November 3, 2022 08:44
@hanzei

hanzei commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

/check-cla

javaguirre
javaguirre approved these changes Nov 3, 2022
View reviewed changes

@javaguirre javaguirre left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM!

@javaguirre
javaguirre self-requested a review November 3, 2022 10:10
@hanzei

hanzei commented Nov 3, 2022

Copy link
Copy Markdown
Contributor

@doc-sheet Would you mind signing the CLA? Please let me know if you have any questions.

@doc-sheet

doc-sheet commented Nov 4, 2022

Copy link
Copy Markdown
Contributor Author

I don't think my request will be approved.

I don't mind if you fix regexp yourself, I just want get working previews for some repos.

@doc-sheet

doc-sheet commented Nov 4, 2022

Copy link
Copy Markdown
Contributor Author

/check-cla

@doc-sheet

doc-sheet commented Nov 4, 2022

Copy link
Copy Markdown
Contributor Author

Hooray!

@hanzei
hanzei requested a review from DHaussermann November 5, 2022 03:39
@hanzei hanzei added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core committer labels Nov 5, 2022
@mattermod

mattermod commented Nov 17, 2022

Copy link
Copy Markdown
Contributor

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

@doc-sheet

doc-sheet commented Dec 8, 2022

Copy link
Copy Markdown
Contributor Author

Hello again.
What should i do next?

@hanzei

hanzei commented Dec 8, 2022

Copy link
Copy Markdown
Contributor

Hey @doc-sheet,

There is nothing from your side that blocks this PR. It's waiting for a QA tester to review it.

@hanzei

hanzei commented Dec 8, 2022

Copy link
Copy Markdown
Contributor

@DHaussermann Gentle reminder to review the PR when you take a look at GitHub again

@DHaussermann

DHaussermann commented Dec 14, 2022

Copy link
Copy Markdown

/update-branch

@spirosoik

spirosoik commented Jan 10, 2023

Copy link
Copy Markdown
Contributor

/update-branch

@hanzei

hanzei commented Feb 20, 2023

Copy link
Copy Markdown
Contributor

/update-branch

@hanzei hanzei added this to the v2.2.0 milestone Feb 20, 2023
@hanzei

hanzei commented Feb 20, 2023

Copy link
Copy Markdown
Contributor

@DHaussermann Gentle reminder to review the PR

@hanzei

hanzei commented Feb 21, 2023

Copy link
Copy Markdown
Contributor

/update-branch

DHaussermann
DHaussermann approved these changes Feb 23, 2023
View reviewed changes

@DHaussermann DHaussermann left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested and passed
Good catch! I can now use plugins that has a . in the name

Briefly regression tested subscription creation to various other repos including more common character like hello-world. No issues found.

LGTM!

Thanks @doc-sheet fir the fix!

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Feb 23, 2023
@hanzei hanzei added the Docs/Not Needed Does not require documentation label Feb 23, 2023
@hanzei
hanzei merged commit 39b1b86 into mattermost:master Feb 23, 2023
@mickmister mickmister mentioned this pull request Mar 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hanzei hanzei hanzei approved these changes
@DHaussermann DHaussermann DHaussermann approved these changes
+1 more reviewer
@javaguirre javaguirre javaguirre approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

4: Reviews Complete All reviewers have approved the pull request Contributor Docs/Not Needed Does not require documentation

Projects

None yet

Milestone

v2.2.0

Development

Successfully merging this pull request may close these issues.

8 participants

@doc-sheet @mattermod @codecov-commenter @hanzei @DHaussermann @spirosoik @javaguirre @mattermost-build