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

feat: support output in both atom and rss2 #100

Merged
merged 19 commits into from Nov 9, 2019

Conversation

@curbengh
Copy link
Contributor

curbengh commented Oct 19, 2019

Fix #59

How to test:

package.json
-  "hexo-generator-feed": "^2.0.0"
+  "hexo-generator-feed": "curbengh/hexo-generator-feed#atom-rss2"
$ rm -rf node_modules/ && npm i
_config.yml
feed:
  type:
    - 'atom'
    - 'rss2'
$ hexo clean && hexo g

cc @mythsman

@curbengh curbengh changed the title Atom rss2 feat: support ouput in both atom and rss2 Oct 19, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Oct 19, 2019

WIP, missing doc and unit test.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Oct 19, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 7f41f0f on curbengh:atom-rss2 into d168026 on hexojs:master.

@curbengh curbengh force-pushed the curbengh:atom-rss2 branch from 417b94b to daa4b32 Oct 19, 2019
@curbengh curbengh marked this pull request as ready for review Oct 20, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Oct 20, 2019

Note that generator.js still output one file only, which is why it's called twice in index.js. This limitation also applies to the unit test, hence it doesn't check both atom and rss2 are generated; at least the test can check the order is correct.

The limitation doesn't apply to autodiscovery, so there is a unit test to check both values exist.


Although the code may be simpler if use type: "both" config, I prefer to explicitly use type: ['atom, 'rss2'] for config readability.

@curbengh curbengh requested a review from hexojs/core Oct 20, 2019
@ertrzyiks ertrzyiks changed the title feat: support ouput in both atom and rss2 feat: support output in both atom and rss2 Oct 21, 2019
@curbengh curbengh requested review from hexojs/core and removed request for hexojs/core Oct 28, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Oct 28, 2019

Ready for review.

@curbengh curbengh force-pushed the curbengh:atom-rss2 branch from af3cbcf to f7e2532 Nov 5, 2019
@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Nov 5, 2019

Rebased. Ready for review.

@tomap
tomap approved these changes Nov 6, 2019
@tomap

This comment has been minimized.

Copy link
Contributor

tomap commented Nov 6, 2019

Maybe add in the doc an example of yaml with both output & both path so people don't struggle to type it

@curbengh

This comment has been minimized.

Copy link
Contributor Author

curbengh commented Nov 9, 2019

Maybe add in the doc an example of yaml with both output & both path

+1. Added.

@SukkaW
SukkaW approved these changes Nov 9, 2019
@curbengh curbengh merged commit 273eb73 into hexojs:master Nov 9, 2019
2 checks passed
2 checks passed
Travis CI - Pull Request Build Passed
Details
coverage/coveralls Coverage remained the same at 100.0%
Details
@curbengh curbengh deleted the curbengh:atom-rss2 branch Nov 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.