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

Fix regression in ./gradlew idea introduced in baseline 3.51.0 #1551

Merged
merged 4 commits into from
Nov 17, 2020

Conversation

iamdanfox
Copy link
Contributor

@iamdanfox iamdanfox commented Nov 17, 2020

Before this PR

@dtobin reported in #dev-foundry-infra that upgrading from baseline 3.50.0 -> 3.51.0 broke ./gradlew idea.

groovy.lang.MissingPropertyException: No such property: DEFAULT_CHECKSTYLE_VERSION for class: com.palantir.baseline.plugins.BaselineCheckstyle
	at groovy.lang.MetaClassImpl.invokeStaticMissingProperty(MetaClassImpl.java:1021)
	at groovy.lang.MetaClassImpl.getProperty(MetaClassImpl.java:1866)
	at groovy.lang.MetaClassImpl.getProperty(MetaClassImpl.java:1842)

Clearly I goofed my original PR here: #1546

After this PR

==COMMIT_MSG==
Fix regression in ./gradlew idea introduced in baseline 3.51.0
==COMMIT_MSG==

I also did a publishToMavenLocal and a manual verification of this, cos I don't really trust our tests for this plugin!

Possible downsides?

@iamdanfox iamdanfox requested a review from CRogers November 17, 2020 17:06
@changelog-app
Copy link

changelog-app bot commented Nov 17, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Fix regression in ./gradlew idea introduced in baseline 3.51.0

Check the box to generate changelog(s)

  • Generate changelog entry

Sorry, something went wrong.

@iamdanfox iamdanfox changed the title Fix regression in ./gradlew idea introduced in baseline 3.51.1 Fix regression in ./gradlew idea introduced in baseline 3.51.0 Nov 17, 2020
then:
BuildResult result = with('idea', '-Didea.active=true', '-is').build()
assert file(".idea/checkstyle-idea.xml").exists() ?: result.output
assert result.task(':idea').outcome == TaskOutcome.SUCCESS ?: result.output
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, I'm actually surprised we got this far without this test.

private void addCopyright(node) {
def copyrightManager = node.component.find { it.'@name' == 'CopyrightManager' }
private void addCopyright(Node node) {
Node copyrightManager = node.component.find {it.'@name' == 'CopyrightManager'}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

a lot of these changes were motivated by @CompileStatic, but I couldn't fully lock it in because of all our xml manipulation. Figured it was better to just get closer 🤷

@@ -345,6 +351,7 @@ class BaselineIdea extends AbstractBaselinePlugin {
return
}

// language=xml
Copy link
Contributor

Choose a reason for hiding this comment

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

One of my fav idea features.

Copy link
Contributor

Choose a reason for hiding this comment

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

TIL!

def copyrightDir = Paths.get("${configDir}/copyright/")
def copyrightFiles = getCopyrightFiles(copyrightDir)
copyrightFiles.each { File file ->
copyrightFiles.each {File file ->
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these whitespace changes are intellij helpfully reformatting things. Given that we don't have a palantir-groovy-format (😂), I couldn't be bothered to revert em all piece by piece

@@ -353,6 +360,7 @@ class BaselineIdea extends AbstractBaselinePlugin {
}

private static void addInspectionProjectProfile(node) {
// language=xml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

i stumbled across this trick further down this file, turns out IntelliJ can give you language-specific code highlighting for snippets!!!
image

@@ -49,7 +49,23 @@ class BaselineIdeaIntegrationTest extends AbstractPluginTest {
buildFile << standardBuildFile

then:
with('idea').build()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this assertion would always pass. the result contained an outcome of either success/fail, but we never checked it. 🤦

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

Successfully merging this pull request may close these issues.

None yet

3 participants