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

get-metadata: missing "Refs:" in metadata section #156

Closed
vsemozhetbyt opened this issue Jan 23, 2018 · 12 comments
Closed

get-metadata: missing "Refs:" in metadata section #156

vsemozhetbyt opened this issue Jan 23, 2018 · 12 comments
Assignees

Comments

@vsemozhetbyt
Copy link
Contributor

For nodejs/node#18310, "Refs:" was omitted in metadata section.

@priyank-p priyank-p added the bug label Jan 23, 2018
@priyank-p
Copy link
Contributor

@vsemozhetbyt can provide more details on this or do you have any clue why that happend?

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Jan 23, 2018

@cPhost I cannot understand the cause.
Commit message:

doc: remove confusing signature in fs.md

Fixes: https://github.com/nodejs/node/issues/18305
Refs: https://github.com/nodejs/node/pull/13424
get-metadata output:
$ winpty get-metadata.cmd 18310
√  Done loading data for nodejs/node/pull/18310
----------------------------------- PR info ------------------------------------
Title      doc: remove confusing signature in fs.md (#18310)
Author     Vse Mozhet Byt <vsemozhetbyt@gmail.com> (@vsemozhetbyt)
Branch     vsemozhetbyt:doc-fs-del-err-signature -> nodejs:master
Labels     doc, fast-track, fs
Commits    1
 - doc: remove confusing signature in fs.md
Committers 1
 - Vse Mozhet Byt <vsemozhetbyt@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/18310
Fixes: https://github.com/nodejs/node/issues/18305
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
--------------------------------------------------------------------------------
√  Requested Changes: 0
√  Approvals: 3, 1 from TSC (joyeecheung)
i  Last Full CI on 2018-01-23T03:47:53Z: https://ci.nodejs.org/job/node-test-pull-request-lite/115/
i  This PR is being fast-tracked
‼  This PR is closed

Seems to be a common case, REFS_RE is matched.

@priyank-p
Copy link
Contributor

Aren't you suppose to add Refs: <url> in for get-metadata it to detect it?

@gibfahn
Copy link
Member

gibfahn commented Jan 23, 2018

Aren't you suppose to add Refs: in for get-metadata it to detect it?

You can add it in the commit message or the first comment in the issue. Looks like @vsemozhetbyt added it to the commit message.

@priyank-p
Copy link
Contributor

Ah yeah, I don't know what i was thinking...

@priyank-p
Copy link
Contributor

priyank-p commented Jan 24, 2018

@joyeecheung it looks this is failing because github is once again using .issue-link or something and the url is not being parsed.

I don't know why this did not came to my mind earlier but we can avoid using jsdom simply by using
bodyText field rather than bodyHTML field. And avoid other bugs caused by it.

@joyeecheung
Copy link
Member

@cPhost I think the OP is about the URL in the commit message not being parsed, rather than the body of the PR?

@priyank-p
Copy link
Contributor

Yep, found that it, we need to also pass commit msg into linkParser and then get refs from one of them. I think

@joyeecheung
Copy link
Member

@cPhost In that case it's not a bug caused by .issue-link, the jsdom parsing is there to work around text like Refs: #XXXXX. We should use something else for commit messages instead.

@priyank-p
Copy link
Contributor

priyank-p commented Jan 25, 2018

@joyeecheung so we can just combine bodyHTML and messageBodyHTML and just passed to LinkParse should fix it.

@github-actions
Copy link
Contributor

This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.

@github-actions github-actions bot added the stale label Aug 17, 2020
@priyank-p
Copy link
Contributor

I think this is no longer an issue since it didn't come up recently.
And, my comments are all but un-understandable back then :(

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

Successfully merging a pull request may close this issue.

5 participants