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

[infrastructure] enable spotless #8068

Closed
wants to merge 3 commits into from
Closed

[infrastructure] enable spotless #8068

wants to merge 3 commits into from

Conversation

J-N-K
Copy link
Member

@J-N-K J-N-K commented Jul 4, 2020

This PR finally enables spotless. After the upgrade to 2.0.0 the memory issues are fixe. The feature-bundles are excludedfor the moment because enabling the extensions for the karaf-maven-plugin currently breaks the build. This seems to be an issue in either the plugin or in maven, as one of them changes the line-endings-property of the build system.

J-N-K added 2 commits July 4, 2020 14:36
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
@J-N-K J-N-K added the infrastructure Build system and Karaf related issues and PRs label Jul 4, 2020
Copy link
Contributor

@sprehn sprehn left a comment

Choose a reason for hiding this comment

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

I am fine with the changes in lgwebos

Signed-off-by: Jan N. Klug <jan.n.klug@rub.de>
@TravisBuddy
Copy link

Hey @J-N-K,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 4287ca80-be09-11ea-a595-8f8781864445

@lolodomo
Copy link
Contributor

lolodomo commented Jul 4, 2020

This will certainly introduce conflicts with my PR #8057 :( If mine could be merged first, I would be happy.

@TravisBuddy
Copy link

Travis tests were successful

Hey @J-N-K,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

Copy link
Contributor

@cpmeister cpmeister left a comment

Choose a reason for hiding this comment

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

You also need to address the conflicts.

Comment on lines +29 to +31
<description>This is the Subscription Key provided by BTicino/Legrand when you subscribe to Smarther - v2.0 product.
Go to
https://developer.legrand.com/tutorials/getting-started/</description>
Copy link
Contributor

@cpmeister cpmeister Jul 8, 2020

Choose a reason for hiding this comment

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

The formatting of the descriptions in this file probably needs to be manually adjusted.

Copy link
Member Author

Choose a reason for hiding this comment

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

Manually adjusting the formatting is exactly what we want to avoid with the spotless formatter, or am I wrong there?

Comment on lines +56 to +59
if (this == object)
return true;
if (object == null || getClass() != object.getClass())
return false;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't if statements have brackets? Are brackets not enforced by spotless?

Copy link
Member Author

Choose a reason for hiding this comment

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

Probably not. But changing that is out-of-scope of this PR.

@wborn
Copy link
Member

wborn commented Aug 24, 2020

Maybe you can fix the merge conflicts so we can merge this @J-N-K?

@@ -76,8 +78,7 @@
<bnd.includeresource>-${.}/NOTICE, -${.}/*.xsd</bnd.includeresource>

<feature.directory>src/main/feature/feature.xml</feature.directory>
<spotless.check.skip>true</spotless.check.skip> <!-- Spotless disabled for now -->
<spotless.version>1.31.1</spotless.version>
<spotless.version>2.0.0</spotless.version>
Copy link
Member

Choose a reason for hiding this comment

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

There are also a few newer versions nowadays, see changelog:

Suggested change
<spotless.version>2.0.0</spotless.version>
<spotless.version>2.0.3</spotless.version>

@J-N-K J-N-K mentioned this pull request Aug 26, 2020
@wborn wborn mentioned this pull request Sep 5, 2020
@wborn
Copy link
Member

wborn commented Sep 5, 2020

Replaced by #8401.

@wborn wborn closed this Sep 5, 2020
@J-N-K J-N-K deleted the spotless branch September 21, 2020 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Build system and Karaf related issues and PRs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants