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: add "Refs:" in metadata #117

Closed
vsemozhetbyt opened this issue Nov 18, 2017 · 8 comments
Closed

get-metadata: add "Refs:" in metadata #117

vsemozhetbyt opened this issue Nov 18, 2017 · 8 comments

Comments

@vsemozhetbyt
Copy link
Contributor

Example: nodejs/node#17107

IIRC, nodejs/node-review adds this data.

@Tiriel
Copy link
Contributor

Tiriel commented Nov 18, 2017

Strange, this feature is already implemented and is supposed to work, but I can't get Refs in metadata either.
Unit tests are even working and properly returning Refs. I'll investigate, if someone doesn't beat me to it.

@joyeecheung
Copy link
Member

node-review doesn't add that refs, actually.

screen shot 2017-11-19 at 12 36 44 am

The link parser code is largely borrowed from node-review. It currently only take urls that are a.issue-link i.e. github issues/PRs

@joyeecheung
Copy link
Member

@Tiriel Did you have a chance to look into this?

@Tiriel
Copy link
Contributor

Tiriel commented Nov 23, 2017

@joyeecheung Not yet, had a pretty busy week. Putting it on my schedule for tonight!

@priyank-p
Copy link
Contributor

The reason Refs is not being inserted to metadata because of

const as = this.OP.querySelectorAll('a.issue-link');

The query selector at line 48 a.issue-link returns undefined because github no longer uses .issue-link class and thats the reason the tests are passing but the feature is not working.

Way to fix this issue is just to remove .issue-link . and remove the unnecessary attributes form test except href just to make the test more readable.

\cc @joyeecheung @Tiriel

Tiriel added a commit to Tiriel/node-core-utils that referenced this issue Nov 23, 2017
@Tiriel
Copy link
Contributor

Tiriel commented Nov 23, 2017

Still looking for a propoer way to parse the links recursively, I'm on it ;)

@joyeecheung
Copy link
Member

@cPhost SGTM

@Tiriel
Copy link
Contributor

Tiriel commented Nov 24, 2017

Don't know what I did last night but I was way too far... Opening a PR. Removing the class was all that was needed it seems. Tests OK and example given OK too.

Tiriel added a commit to Tiriel/node-core-utils that referenced this issue Nov 24, 2017
priyank-p pushed a commit that referenced this issue Nov 29, 2017
* links: removed unused class, allows parsing non github refs

Fixes: #117

* test: added test for new refs parsing
johnfrench3 pushed a commit to johnfrench3/core-utils-node that referenced this issue Nov 2, 2022
* links: removed unused class, allows parsing non github refs

Fixes: nodejs/node-core-utils#117

* test: added test for new refs parsing
renawolford6 added a commit to renawolford6/node-dev-build-core-utils that referenced this issue Nov 10, 2022
* links: removed unused class, allows parsing non github refs

Fixes: nodejs/node-core-utils#117

* test: added test for new refs parsing
Developerarif2 pushed a commit to Developerarif2/node-core-utils that referenced this issue Jan 27, 2023
* links: removed unused class, allows parsing non github refs

Fixes: nodejs/node-core-utils#117

* test: added test for new refs parsing
gerkai added a commit to gerkai/node-core-utils-project-build that referenced this issue Jan 27, 2023
* links: removed unused class, allows parsing non github refs

Fixes: nodejs/node-core-utils#117

* test: added test for new refs parsing
shovon58 pushed a commit to shovon58/node-core-utils that referenced this issue Jun 9, 2023
* links: removed unused class, allows parsing non github refs

Fixes: nodejs/node-core-utils#117

* test: added test for new refs parsing
patrickm68 added a commit to patrickm68/NodeJS-core-utils that referenced this issue Sep 14, 2023
* links: removed unused class, allows parsing non github refs

Fixes: nodejs/node-core-utils#117

* test: added test for new refs parsing
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

No branches or pull requests

4 participants