Skip to content
This repository has been archived by the owner on Nov 15, 2017. It is now read-only.

rename sample files once unzipped #132

Merged
merged 4 commits into from
Jun 19, 2017
Merged

rename sample files once unzipped #132

merged 4 commits into from
Jun 19, 2017

Conversation

NathanPJF
Copy link
Collaborator

What are you trying to accomplish with this PR?

Companion piece to this PR in slate: Shopify/slate#179

The idea: After running slate theme, you are given an unzipped slate-src.zip plus two files config-sample and .gitignore-sample. Let's save the manual effort of renaming both these files when you start off.

Checklist

For contributors:

  • I have updated the documentation in the README file to reflect these changes, if applicable.

For maintainers:

  • I have 🎩'd these changes.
  • I have bumped the package.json version in a separate PR, if applicable.

Considerations:

@NathanPJF NathanPJF requested a review from bertiful June 16, 2017 18:49
@@ -81,6 +81,12 @@ export default function(program) {

return null;
})
.then(() => {
renameFile(join(root, 'config-sample.yml'), join(root, 'config.yml'));
renameFile(join(root, '.gitignore-sample', join(root, '.gitignore');
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing closing ")".

src/utils.js Outdated
* @param {string} target - The renamed path of the file.
*/
export function renameFile(current, target) {
rename(current, target, (err) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be wrapping this in return new Promise((resolve, reject) => {});?

src/utils.js Outdated
export function renameFile(current, target) {
rename(current, target, (err) => {
if (err) {
throw err;
Copy link
Contributor

Choose a reason for hiding this comment

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

If so, reject(err) here. And resolve() outside of the condition.

@shopify-admins shopify-admins requested a review from bertiful June 16, 2017 21:48
@NathanPJF
Copy link
Collaborator Author

NathanPJF commented Jun 16, 2017

New project... that fyre. 🔥

@@ -89,7 +89,6 @@ export default function(program) {
];

function renamePromiseFactory({oldFile, newFile, dir}) {
console.log(` Renaming ${oldFile} to ${newFile}...`);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Decided I didn't like the message after it said "you're done".

Copy link
Contributor

Choose a reason for hiding this comment

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

Good call removing this.

{oldFile: '.gitignore-sample', newFile: '.gitignore', dir: root},
];

function renamePromiseFactory({oldFile, newFile, dir}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for this ES6 lesson 🥇

@NathanPJF NathanPJF merged commit ba73636 into master Jun 19, 2017
@NathanPJF NathanPJF deleted the write-ignore-file branch June 19, 2017 21:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants