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

Rename sourcesFile to sourcesJson #160

Merged

Conversation

cprussin
Copy link
Contributor

@cprussin cprussin commented Dec 9, 2019

See #159 (comment). I opted to rename the option rather than updating the docs because the cli option is --sources-json and so using an option sourcesJson is more obvious than keeping it as sourcesFile.

@nmattia nmattia force-pushed the cprussin/fix-custom-sources-option-docs branch from 420d9a2 to f518e2e Compare December 9, 2019 09:20
@nmattia
Copy link
Owner

nmattia commented Dec 9, 2019

Thanks! I pushed some CI fixes to your branch, the tests weren't being run on PRs of non-collaborators. As you can see the tests fail, the reason is that we version every change in sources.nix. I think the simplest solution is to keep the name sourcesFile and rename the CLI arg to --sources-file. That's also more in line with what we discussed in #159 since there'll soon be a single "sources" file (sources.nix will disappear).

@cprussin
Copy link
Contributor Author

cprussin commented Dec 9, 2019

@nmattia done, let me know if this is what you had in mind

@nmattia
Copy link
Owner

nmattia commented Dec 9, 2019

@cprussin awesome, thanks!

@nmattia nmattia force-pushed the cprussin/fix-custom-sources-option-docs branch from 3f9e7d5 to f3202a1 Compare December 9, 2019 17:44
@nmattia
Copy link
Owner

nmattia commented Dec 9, 2019

I've amended the CHANGELOG, hope you don't mind! I'll merge as soon as this is green.

@nmattia nmattia merged commit e5a3c9a into nmattia:master Dec 9, 2019
@cprussin cprussin deleted the cprussin/fix-custom-sources-option-docs branch December 9, 2019 18:38
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