Skip to content

Added the bulk copy to remote resources#377

Merged
SBoudrias merged 2 commits intoyeoman:masterfrom
wesleytodd:remote-bulk-copy
Nov 5, 2013
Merged

Added the bulk copy to remote resources#377
SBoudrias merged 2 commits intoyeoman:masterfrom
wesleytodd:remote-bulk-copy

Conversation

@wesleytodd
Copy link
Contributor

This is a continuation of #350 & #359. Added the new bulk operations to the remote resources. I had to mess with the way that the source root was used in the original implementation because the conflicter wait's till next tick to run, which reset the sourceRoot before the copy actually took place. But other than that it is just a proxy to the new methods.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.41%) when pulling 781fd0d on wesleytodd:remote-bulk-copy into 53eafa6 on yeoman:master.

@wesleytodd
Copy link
Contributor Author

Would you guys care if I snuck some other tests for the remote functionality into this PR? It looks like I could copy and paste my new tests but have them call the standard remote methods and get better coverage in there. But if I did two PR's I would have conflicts. It is not a big deal for me, but it would make them harder to merge for you. Just wondering...

Copy link
Member

Choose a reason for hiding this comment

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

typo "becuase"

@SBoudrias
Copy link
Member

Would you guys care if I snuck some other tests for the remote functionality into this PR?

Not at all, add a new commit with more test if that works for you.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.05%) when pulling 755f2fd on wesleytodd:remote-bulk-copy into 53eafa6 on yeoman:master.

@SBoudrias
Copy link
Member

Hey, this looks good to me. Only changes, is that I'll made the describe more generic. Instead of remote.bulkCopy(...., just named them #bulkCopy. It's easier to target with grep and there's less chance of the doc string becoming outdated after changes. Checkout refactor did over the test/env.js to grasp the style: https://github.com/yeoman/generator/blob/master/test/env.js

Otherwise, that looks good to merge for the 0.14.0-rc2

@SBoudrias SBoudrias mentioned this pull request Oct 28, 2013
@SBoudrias
Copy link
Member

@wesleytodd You around? I'd like to merge this in before 0.14

@wesleytodd
Copy link
Contributor Author

Hey @SBoudrias, sorry I have been busy with work the past week. I can probably do that by wednesday night if that works for you. Sorry about being un-responsive, but as I am sure you know, paying work comes first.

@SBoudrias SBoudrias merged commit 755f2fd into yeoman:master Nov 5, 2013
@SBoudrias
Copy link
Member

Hey, I merged it in as is. That's mostly because, I played around this test file a bit, and it is quite ugly... It'd need a major restructure and rethink to make each test independant. Soooo, I told myself, lets merge and see to improve that later on.

Thanks for the good work!

@wesleytodd
Copy link
Contributor Author

Thanks @SBoudrias. I agree that that file could use some refactoring, just writing the bulk tests was a pain because of the tests that were trying to copy the entire fixtures directory. Anyway, glad I could help.

@wesleytodd wesleytodd deleted the remote-bulk-copy branch November 6, 2013 03:57
@sindresorhus
Copy link
Member

Thanks @wesleytodd :)

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.

4 participants