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

[test] Fix prettier ci task with multiple changed files #12579

Merged
merged 1 commit into from
Aug 19, 2018

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Aug 18, 2018

Fixes various issues in #12564:

  • no changes caused grep to match everything (this is a non-issue because why would this trigger a CI build?)
  • more than one changed file broke the if statement (I'm concerned that an error in a bash command does not cause the CI task to fail)
  • passing more than one file arg to the cli was considered on file

Sorry about the inconvenience. I should've been more careful with languages that I'm not that familiar with.

The build should be considered broken. Once this is confirmed I will retrigger with only one file changed and the remove the broken format completely. Only if all those builds return as expected should this be merged.

@eps1lon eps1lon changed the title Fix ci prettier [test] Fix prettier ci task with multiple changed files Aug 18, 2018
@eps1lon
Copy link
Member Author

eps1lon commented Aug 18, 2018

This does work locally git diff --name-only master... returns different results in CI. I'm going to investigate how I can diff against master in CircleCI.

@eps1lon
Copy link
Member Author

eps1lon commented Aug 18, 2018

The current approach relies on CIRCLE_COMPARE_URL which is empty at the moment. No idea why (it was set on previous builds).

I have no idea how to get a commit range from master to branch in CircleCI otherwise. #12564 should be reverted if no one else has solution.

@eps1lon eps1lon force-pushed the fix-ci-prettier branch 3 times, most recently from 9b25d39 to 5e5968c Compare August 18, 2018 18:39
@oliviertassinari
Copy link
Member

I have no idea how to get a commit range from master to branch in CircleCI

The test_material-ui-x test can help.

@eps1lon eps1lon force-pushed the fix-ci-prettier branch 3 times, most recently from 7b51d55 to c37d493 Compare August 19, 2018 12:51
@eps1lon
Copy link
Member Author

eps1lon commented Aug 19, 2018

Thanks for the hint @oliviertassinari. Responsible for the unexpected git diff master behavior is the hard reset that CircleCI does when checking out the code.

I had to work around CircleCIs missing support for interpolation while setting environment variables quite a bit.

The prettier step now exited as expected on the following test case:

@oliviertassinari oliviertassinari merged commit 823fed0 into mui:master Aug 19, 2018
@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 19, 2018

Thanks. I wish we had a detailed diff of what's wrong (like before) rather than just the files that need to be fixed, but it's not that important (easier to graspe for a new contributor). Let's try that out.

@eps1lon
Copy link
Member Author

eps1lon commented Aug 20, 2018

While this is useful for linting since most of those issues need to be resolved manually formatting should not be done manually. That should either be handled by the IDE or via yarn prettier.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants