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

Add acceptance tests for the mocha blueprints #4187

Merged
merged 1 commit into from
Feb 28, 2016

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Feb 25, 2016

No description provided.

@Turbo87
Copy link
Member Author

Turbo87 commented Feb 25, 2016

It looks like the blueprints on Windows are actually being generated with \r\n instead of \n on AppVeyor due to https://github.com/emberjs/data/blob/master/appveyor.yml#L3-L5 which is causing trouble with the tests, but the same is also true for the existing QUnit blueprints.

Should I adjust the tests to not test the line breaks for now or change AppVeyor to always use the original line breaks?

/cc @stefanpenner @kellyselden

@Turbo87
Copy link
Member Author

Turbo87 commented Feb 26, 2016

rebased on top of #4189 to solve the line ending issues

@bmac
Copy link
Member

bmac commented Feb 26, 2016

Do you mind rebasing this pr @Turbo87

@Turbo87
Copy link
Member Author

Turbo87 commented Feb 26, 2016

@bmac done

@pangratz
Copy link
Member

@Turbo87 this looks good. Can you please squash into 1 commit?

Update "ember-cli-blueprint-test-helpers" to v0.9.0

node-tests/blueprints/model: Add test for "mocha" blueprint

node-tests/blueprints/adapter: Add test for "mocha" blueprint

node-tests/blueprints/serializer: Add test for "mocha" blueprint

node-tests/blueprints/transform: Add test for "mocha" blueprint
@Turbo87
Copy link
Member Author

Turbo87 commented Feb 28, 2016

@pangratz done

pangratz added a commit that referenced this pull request Feb 28, 2016
Add acceptance tests for the mocha blueprints
@pangratz pangratz merged commit 03bd070 into emberjs:master Feb 28, 2016
@pangratz
Copy link
Member

🎉

@Turbo87
Copy link
Member Author

Turbo87 commented Feb 28, 2016

@pangratz out of curiosity, what is the reason for squashing the commits all the time?

@Turbo87 Turbo87 deleted the mocha-blueprint-tests branch February 28, 2016 23:09
@pangratz
Copy link
Member

I can only speak for myself but I can think of the following reasons:

  • easier possibility to back port stuff into other release channels and older releases via a single cherry-pick
  • cleaner to revert if stuff breaks
  • one PR handles one specific issue in one commit
  • cleaner git log as every entry corresponds to one single, cohesive change
  • consistency, since almost all PR's in this repo are handled this way

@Turbo87
Copy link
Member Author

Turbo87 commented Feb 29, 2016

easier possibility to back port stuff into other release channels and older releases via a single cherry-pick

👍 although it would manageable 😉

cleaner to revert if stuff breaks

GitHub actually allows you to just revert the merge commit

one PR handles one specific issue in one commit

I personally try to keep the commits as small as possible to make them easier to review and then limit the PRs to single, specific features

cleaner git log as every entry corresponds to one single, cohesive change

granted, although since we're still using non-fast-forward merge commits the history graph isn't entirely clean anyway

consistency, since almost all PR's in this repo are handled this way

sure, that's essentially the reason why I'm not complaining, just asking 😉

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.

None yet

3 participants