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

YAML implementation with Jackson #1478

Merged
merged 12 commits into from
Jan 12, 2023

Conversation

blacelle
Copy link
Contributor

This follows #910.

It enables YAML formatting with Jackson

@blacelle blacelle changed the title Base implementation YAML implementation with Jackson Jan 10, 2023
Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

Wow, you're on a roll! Needs

  • no default includes
  • add an entry to the giant table in the root README.md
    • be sure to run Spotless in Java 11 or less so that freshmark works
  • take a look at the compileOnly thing, but no change is required

plugin-maven/README.md Outdated Show resolved Hide resolved
@blacelle
Copy link
Contributor Author

blacelle commented Jan 11, 2023

This YAML integration comes with multiple limitations.

@nedtwigg Would this remain acceptable, as long as these limitations are clearly documented?

@nedtwigg
Copy link
Member

If this YAML formatter will be useful to you, then I want to merge it! If these limitations mean that it won't work for you, then I'd rather not merge it. I love to merge things that help people solve their own problems, and I'm skeptical of features that the PR author doesn't need, but just imagines other future users possibly needing.

The tests are all still failing. Hopefully we can fix them up and cut a release tomorrow :)

@blacelle
Copy link
Contributor Author

Some notes for myself and other maven-preferred developper:

  1. mvn tests can be run from the root with ./gradlew :plugin-maven:check --build-cache
  2. It is unclear what cd plugin-maven && ../gradlew publishToMavenLocal will push in your local repository (cache issue?)
  3. cd plugin-maven && ../gradlew test --tests '*yaml*' will trigger some tests, but it is unclear on which code they apply (cache issue?)
  4. ./gradlew :plugin-maven:runMavenBuild :plugin-maven:test --tests '*yaml*' --no-build-cache seems to correctly discard the cached mvn plugin, and execute tests on the latest code.

@blacelle
Copy link
Contributor Author

blacelle commented Jan 12, 2023

Wow. This was a burden, but now things are OK. These caches issues related to maven were a major difficulty.

We may invest in maven-harness:

	<dependency>
		<!-- https://maven.apache.org/plugin-testing/maven-plugin-testing-harness/getting-started/index.html -->
		<groupId>org.apache.maven.plugin-testing</groupId>
		<artifactId>maven-plugin-testing-harness</artifactId>
		<version>3.3.0</version>
		<scope>test</scope>
	</dependency>

in order not to execute tests through forked mvn commands (and deployment in a tmp repository. ). We may also imvest in unit-tests not plugin-based.

JSON integration was KO. I got caught by unexpected behavior of json-simple (which kind-of sortKeys by default, but in a different fashion than Gson when the option is on).

@blacelle blacelle marked this pull request as ready for review January 12, 2023 07:09
@blacelle blacelle force-pushed the IntroduceYamlJvmBased_withJackson branch from 9129c3b to 13d9d3c Compare January 12, 2023 07:10
@blacelle blacelle requested a review from nedtwigg January 12, 2023 07:55
Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

Thanks for sticking with this! I think it's time for the maven build to get fixed up. Subscribe to #554, I'll have something to share there shortly.

@nedtwigg nedtwigg merged commit 07bafaf into diffplug:main Jan 12, 2023
nedtwigg added a commit that referenced this pull request Jan 12, 2023
@nedtwigg
Copy link
Member

Published in plugin-maven 2.30.0.

@blacelle blacelle deleted the IntroduceYamlJvmBased_withJackson branch January 15, 2023 12:18
benkard added a commit to benkard/mulkcms2 that referenced this pull request Apr 2, 2023
…2.0 (mulk/mulkcms2!16)

This MR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [com.diffplug.spotless:spotless-maven-plugin](https://github.com/diffplug/spotless) | build | minor | `2.31.0` -> `2.32.0` |

---

### Release Notes

<details>
<summary>diffplug/spotless</summary>

### [`v2.32.0`](https://github.com/diffplug/spotless/blob/HEAD/CHANGES.md#&#8203;2320---2023-01-13)

##### Added

-   Add option `editorConfigFile` for `ktLint` [#&#8203;142](diffplug/spotless#142)
    -   **POTENTIALLY BREAKING** `ktlint` step now modifies license headers. Make sure to put `licenseHeader` *after* `ktlint`.
-   Added `skipLinesMatching` option to `licenseHeader` to support formats where license header cannot be immediately added to the top of the file (e.g. xml, sh). ([#&#8203;1441](diffplug/spotless#1441)).
-   Add YAML support through Jackson ([#&#8203;1478](diffplug/spotless#1478))
-   Added support for npm-based [ESLint](https://eslint.org/)-formatter for javascript and typescript ([#&#8203;1453](diffplug/spotless#1453))
-   Better suggested messages when user's default is set by JVM limitation. ([#&#8203;995](diffplug/spotless#995))

##### Fixed

-   Support `ktlint` 0.48+ new rule disabling syntax ([#&#8203;1456](diffplug/spotless#1456)) fixes ([#&#8203;1444](diffplug/spotless#1444))
-   Fix subgroups leading catch all matcher.

##### Changes

-   Bump default version for `prettier` from `2.0.5` to `2.8.1` ([#&#8203;1453](diffplug/spotless#1453))
-   Bump the dev version of Gradle from `7.5.1` to `7.6` ([#&#8203;1409](diffplug/spotless#1409))
    -   We also removed the no-longer-required dependency `org.codehaus.groovy:groovy-xml`
-   Breaking changes to Spotless' internal testing infrastructure `testlib` ([#&#8203;1443](diffplug/spotless#1443))
    -   `ResourceHarness` no longer has any duplicated functionality which was also present in `StepHarness`
    -   `StepHarness` now operates on `Formatter` rather than a `FormatterStep`
    -   `StepHarnessWithFile` now takes a `ResourceHarness` in its constructor to handle the file manipulation parts
    -   Standardized that we test exception *messages*, not types, which will ease the transition to linting later on
    -   Bump default `ktlint` version to latest `0.47.1` -> `0.48.1` ([#&#8203;1456](diffplug/spotless#1456))
-   Switch our publishing infrastructure from CircleCI to GitHub Actions ([#&#8203;1462](diffplug/spotless#1462)).
    -   Help wanted for moving our tests too ([#&#8203;1472](diffplug/spotless#1472))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNC4yNC4wIiwidXBkYXRlZEluVmVyIjoiMzQuMjQuMCJ9-->
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.

2 participants