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

Switch to Spotless Groovy formatter #843

Closed
1 task
marcphilipp opened this issue May 8, 2017 · 15 comments
Closed
1 task

Switch to Spotless Groovy formatter #843

marcphilipp opened this issue May 8, 2017 · 15 comments

Comments

@marcphilipp
Copy link
Member

Overview

We currently use a custom formatting config for Groovy source files. Now that diffplug/spotless#13 is fixed, we should switch to use GrEclipse formatting for Groovy files.

Deliverables

  • Change Spotless config to use Groovy extension with GrEclipse formatter settings
@fvgh
Copy link

fvgh commented May 9, 2017

If you like can provide you with a proposal (PR) next week. We would need to switch to spotless 3.3.2, since I will need the fix for diffplug/spotless#106.

@marcphilipp
Copy link
Member Author

That would be great! 👍

@fvgh
Copy link

fvgh commented May 9, 2017

Happy that you are interested in my work.
But I just used is on a very small code base till now.
Thanks for volunteering to be my guinea pig 😉.
Just kidding. I will have a careful look, but it might be possible that I find some issues, so that we need to postpone this issue if I still need to fix some minor things in my diffplug/spotless#13 implementation.

@marcphilipp
Copy link
Member Author

Sure, no worries.

@fvgh
Copy link

fvgh commented Jun 10, 2017

@marcphilipp Sorry Marc, I am not familiar with the JUnit PR cycle. Is there anything your are waiting for regarding #860? I currently waiting for a feed back whether the proposal is as such acceptable before applying the changes agreed upon (like this finding).

@marcphilipp
Copy link
Member Author

@fvgh Sorry for the delay! We were just busy with other things. I've reviewed #860 now.

@fvgh
Copy link

fvgh commented Jun 10, 2017

@marcphilipp I did not want to rush the things, was just not sure whether you guys waited for something I should do. Thanks for the review. There is something I want to check on the auto paddedCell feature applied by Spotless, before I answer your review comments.

@marcphilipp
Copy link
Member Author

Sure, no worries.

@fvgh
Copy link

fvgh commented Jun 10, 2017

Last question: Are you (in a separated PR) interested to apply the formatter on the Gradle files as well?

@marcphilipp
Copy link
Member Author

Apply the formatter? I thought that was what #860 is doing, isn't it? I'm sure I'm missing something. Please try to clarify. 🙂

@fvgh
Copy link

fvgh commented Jun 10, 2017

Currently it only selects the groovy files.
Question is, whether you want to have a (slightly different) configuration for *.gradle as well.

@marcphilipp
Copy link
Member Author

Sorry, I mixed up Gradle and Groovy... 🙈

A formatting check for Gradle files sounds like a good idea! Please open a new issue/PR for that if you have time!

@fvgh
Copy link

fvgh commented Jun 10, 2017

Ok. Will come back to you on #860 next week (no time this weekend). When this issue is solved to everybody's convenience, I'll provide you with another PR. The difference between groovy and gradle is just that the import sorter and license header check needs to be removed.

@fvgh
Copy link

fvgh commented Jun 12, 2017

@marcphilipp I am not so firm with the review process. Do you expect my changes as another commit so that you can see differences, or do you prefer a squashed PR to keep the history clean?

@marcphilipp
Copy link
Member Author

Feel free to push additional commits to the existing branch. We can squash it when we merge it if we see fit.

@marcphilipp marcphilipp modified the milestones: 5.0 M5, General Backlog Jun 16, 2017
@fvgh fvgh mentioned this issue Jul 2, 2017
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants