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.

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

Mentions in Changelog send notifications #1421

Closed
jrjohnson opened this issue Oct 2, 2019 · 5 comments · Fixed by #1426
Closed

Mentions in Changelog send notifications #1421

jrjohnson opened this issue Oct 2, 2019 · 5 comments · Fixed by #1426

Comments

@jrjohnson
Copy link

Seems to be a recurrence of #427 except for Changelog summary. As an example this PR rdohms/chainlink#36 mentions me in the Changelog section and I got notified about it

@feelepxyz
Copy link
Contributor

@jrjohnson yes will take a look at fixing this!

@pedropombeiro
Copy link

pedropombeiro commented Oct 3, 2019

Curiously, only the first part of the string up to the first code fence (```registerLoader```) seems to have been sanitized (see the body element in https://api.github.com/repos/rdohms/chainlink/issues/36). This seems to point to a bug in the logic in sanitize_links_and_mentions, where instead of iterating through all the code fences, it considers everything after the start of the first code fence to be part of the code fence, and therefore ingests it as-is.

I've added a couple of tests in my fork that demonstrate the bug. If I have some spare time I'll take a look into fixing the implementation.

@feelepxyz
Copy link
Contributor

@pombeirp yup nice find, I've got a fix here: #1426

@pedropombeiro
Copy link

Thanks @feelepxyz! I'll create a separate PR to handle an additional scenario with complex nested code fences too, to avoid false positives.

@jrjohnson
Copy link
Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants