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

Preserve DOCTYPE in checkstyle.xml #1606

Merged
merged 3 commits into from
Jan 7, 2021
Merged

Preserve DOCTYPE in checkstyle.xml #1606

merged 3 commits into from
Jan 7, 2021

Conversation

pkoenig10
Copy link
Member

@pkoenig10 pkoenig10 commented Jan 7, 2021

Before this PR

The XML parsing/writing logic added in #1603 doesn't preserve the DOCTYPE. This causes the checkstyleMainCircleFinalizer task to fail.

https://app.circleci.com/pipelines/github/palantir/gradle-baseline/1699/workflows/2623eafb-018b-41d4-bd1f-9fb2e6511f37/jobs/23047

After this PR

Preserve the DOCTYPE when writing the checkstyle XML.

@changelog-app
Copy link

changelog-app bot commented Jan 7, 2021

Generate changelog in changelog/@unreleased

Type

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

Description

Baseline correctly preserves the DOCTYPE when generating checkstyle.xml.

Check the box to generate changelog(s)

  • Generate changelog entry

@policy-bot policy-bot bot requested a review from iamdanfox January 7, 2021 19:30
Authors: Robert Fink, Brian Worth, Merrick Zoubeiri, and many other contributors. Based in part on http://checkstyle.sourceforge.net/google_style.html
Please keep checks alphabetized with one exception: "relaxed" checks are grouped together at the bottom for easier disabling.
Check-specific comments reference documents internal to Palantir and can be safely ignored or removed.
-->
Copy link
Member Author

@pkoenig10 pkoenig10 Jan 7, 2021

Choose a reason for hiding this comment

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

Removed this comment because it didn't write well - it ends up looking like:

<?xml version="1.0" encoding="UTF-8" standalone="no"?><!--
    Palantir Baseline Checkstyle configuration.
    Authors: Robert Fink, Brian Worth, Merrick Zoubeiri, and many other contributors. Based in part on http://checkstyle.sourceforge.net/google_style.html
    Please keep checks alphabetized with one exception: "relaxed" checks are grouped together at the bottom for easier disabling.
    Check-specific comments reference documents internal to Palantir and can be safely ignored or removed.
 --><module name="Checker">

This comment isn't particularly useful. In particular, the checks are no longer alphabetized.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'd prefer not to make this change in the same PR as a bugfix.

Copy link
Member Author

Choose a reason for hiding this comment

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

Are you opposed to the change? I'm happy to make another PR, but if you're just going to approve that one it seems a bit silly.

I probably should have removed this in #1603.

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't really have an opinion one way or the other on the comment.

@bulldozer-bot bulldozer-bot bot merged commit 7080a91 into develop Jan 7, 2021
@bulldozer-bot bulldozer-bot bot deleted the pkoenig/xml branch January 7, 2021 20:24
@svc-autorelease
Copy link
Collaborator

Released 3.63.3

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.

3 participants