fix: link cannot be resolved #89

[kahosan]

fix: #89

[kahosan]

maybe max-lines should be a bit longer?

[ndaidong]

@kahosan thank you for your contribution.

What you committed are basically good in logic. However there is some issues with coding convention need to get fixed.

Also, I have other ideas for your code:

1, The variable named hostname is not very suitable

At least in the context of URL object in JavaScript, where hostname usually does not contain protocol.
In our case, it should be called baseUrl or something like that, to avoid confusion.

2, In the extract() method, parsing the hostname/baseUrl from feed url may lead to problem.

Sometimes the feed url may be different from the actual RSS resource. For example the feed urls from feedblitz, or feedburner. That's when the logic fails.

Let's try the following feeds:

• https://feeds.feedburner.com/MachineLearningMastery
• https://feeds.feedblitz.com/baeldung/cs&x=1

3, In the extractFromJson() and extractFromXml() methods, you pass hostname/baseUrl via the third parameter, which is not the best way,

With 2 and 3, I recommend to use the parameter parserOptions for all methods, to keep thing simple and consistent.

For example:

extract('https://feeds.feedburner.com/MachineLearningMastery', {
  baseUrl: 'https://machinelearningmastery.com/',
})

4, You can make use of the method absolutify() in utils/linker.js

https://github.com/extractus/feed-extractor/blob/main/src/utils/linker.js#L12

[ndaidong]

max-lines

yes, you can increase that number in the eslint config file.

[kahosan]

@ndaidong Should we let users fill in the baseUrl themselves?

[ndaidong]

@ndaidong Should we let users fill in the baseUrl themselves?

yes, I think so. We only use this when encountering non-standard feeds.

[ndaidong]

@kahosan I've released v6.2.3 with your latest updates. Thank you.

[coveralls]

Pull Request Test Coverage Report for Build 5262712919

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

• For more information on this, see Tracking coverage changes with pull request builds.
• To avoid this issue with future PRs, see these Recommended CI Configurations.
• For a quick fix, rebase this PR at GitHub. Your next report should be accurate.

Details

• 7 of 7 (100.0%) changed or added relevant lines in 3 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.8%) to 84.296%

---

Totals
Change from base Build 5262148478:	0.8%
Covered Lines:	198
Relevant Lines:	198

---

💛 - Coveralls