Skip to content

Specify filename with CLI options#6

Merged
mantoni merged 2 commits intojavascript-studio:masterfrom
crookedneighbor:specify_filename_with_options
Jan 10, 2017
Merged

Specify filename with CLI options#6
mantoni merged 2 commits intojavascript-studio:masterfrom
crookedneighbor:specify_filename_with_options

Conversation

@crookedneighbor
Copy link
Copy Markdown
Contributor

Set up changes to be an instance of a Changes function. This is technically a breaking change if this module is being used programatically. If you'd prefer not to do a major version bump, I can rewrite it to not be a breaking change, but the code is messier (as you either need to pass in the file name to both methods, or provide some new method to set a variable for the file name.

Also added a help message.

@crookedneighbor
Copy link
Copy Markdown
Contributor Author

I recommend using the no white space link (https://github.com/javascript-studio/studio-changes/pull/6/files?w=1) to review, so it's easier to parse the actual changes.

@mantoni
Copy link
Copy Markdown
Member

mantoni commented Jan 3, 2017

Thanks for the pull request!

I'm not concerned about breaking the API. It's not yet officially documented.

However, I'd rather simply treat the module as the single instance that holds all state. While this might be bad design in other cases, the purpose of this module is a vey focused and short lived task. So, how about just adding a function to change the default file name?

@crookedneighbor
Copy link
Copy Markdown
Contributor Author

Sure. Will try to do it this week.

@crookedneighbor
Copy link
Copy Markdown
Contributor Author

I've made the requested changes. Let me know if you need anything else.

bin/cmd.js Outdated

Options are ...
-f, --file [FILENAME] Specify the name of the changelog file. Defaults to CHANGES.md.
-h, --help Dispaly this help message.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Typo: "Display"

`;
/* eslint-enable */

if (argv.h || argv.help) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Did you know that minimist has an alias option?

@mantoni
Copy link
Copy Markdown
Member

mantoni commented Jan 8, 2017

Thanks, also for adding the --help option. This is great 👍

I'd like to keep your separate commits for the help message and the file option. Would you mind removing the first, obsolete commit? Let me know if I can help with editing the history.

@crookedneighbor
Copy link
Copy Markdown
Contributor Author

Typo fixed and commits re-written. Let me know if you need anything else.

@mantoni mantoni merged commit 56226f5 into javascript-studio:master Jan 10, 2017
@mantoni
Copy link
Copy Markdown
Member

mantoni commented Jan 10, 2017

Thanks a lot, great work! Released in v1.1.0.

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.

2 participants