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

Fix: Changelog #800

Merged
merged 2 commits into from
Sep 5, 2019
Merged

Fix: Changelog #800

merged 2 commits into from
Sep 5, 2019

Conversation

sarvaje
Copy link
Contributor

@sarvaje sarvaje commented Aug 26, 2019

Fix #799

Pull request checklist

Make sure you:

Short description of the change(s)

@sarvaje sarvaje requested a review from antross August 26, 2019 23:47
@@ -724,7 +724,7 @@ const createHintCategories = (files) => {
};

const updateChangelog = async () => {
const files = await globby([`${constants.dirs.HINT_PACKAGES}/*/CHANGELOG.md`]);
const files = await globby([`${constants.dirs.NODE_MODULES}/{hint,@hint/{${resources.join(',')},configuration-*/node_modules/@hint/{${resources.join(',')}}}-*}/CHANGELOG.md`]);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably applies elsewhere in this file, but shouldn't all resources be under either the root node_modules or under node_modules/configuration-all/node_modules? In other words, do we need to search all configurations?

On a related note, is there any reason we're excluding configurations themselves by limiting this to @hint/{connector-*, extension-*, formatter-*, hint-*, parser-*} versus grabbing all @hint/* packages (also probably applies elsewhere in this file)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we weren't including configuration to the documentation, but I think we should include it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

versus grabbing all @hint/*

I think we don't want to grab @hint/utils*.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. Though perhaps we should just be getting the list from configuration-all? That should already be excluding the packages we don't want. Then we're not duplicating the filtering logic here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep, I'm working on the configuration-all thing.

@antross antross merged commit fefa459 into webhintio:master Sep 5, 2019
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 this pull request may close these issues.

500 server error on changelog page
3 participants