-
Notifications
You must be signed in to change notification settings - Fork 145
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(init): creates the contributor file if not found #158
Conversation
If not found, the specified contributor file (defaulting to `README.md`) will be created prior to injections in it. re all-contributors/app#105
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to add a test for this?
Do you think we need tests?
@jakebolam It would be wiser and better to add them (especially as it will rebalance the coverage). Edit: I've added some test cases. |
Renamed `check-file` to `file-exist` and its exported function to `ensureFileExists`
Added two test cases for the contributors file existence check
@jakebolam I've added tests by the way. |
@@ -1,4 +1,6 @@ | |||
import {unlink} from 'fs' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[minor/non blocking] would be good if we could mock the file system using something like https://www.npmjs.com/package/mock-fs
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've tried mock-fs
which works fine for testing files that already exist but for those who don't (i.e. LOREM.md
) which are meant to be deleted as soon as the test suite finishes, it's not doable as the code uses fs
to write to files (when it doesn't exist) and mock-fs
doesn't have any unlink
like method.
Mind you, I could get around that by using fs.unlink
but I'm not sure if it would be worthwhile with mock-fs
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tbenning What do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jakebolam Should I go ahead and merge this or look into another option to mock the FS?
🎉 This PR is included in version 6.4.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What:
Resolve partly all-contributors/app#105
Why:
Because projects relying on this one (such as
all-contributors-bot
) would need to have a way to get the contributors' file generated if it didn't exist during the initialisation process, thus allowing the injection of the contributors' section.How:
Checks the file system for the presence of said contributors file and creates it if it doesn't.
Checklist: