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

more flexibility in keeping "sloppy" parts of existing license header #1611

Open
3 of 6 tasks
planetf1 opened this issue Mar 7, 2023 · 4 comments
Open
3 of 6 tasks

Comments

@planetf1
Copy link

planetf1 commented Mar 7, 2023

If you are submitting a bug, please include the following:

  • summary of problem
  • gradle or maven version
  • spotless version
  • operating system and version
  • copy-paste your full Spotless configuration block(s), and a link to a public git repo that reproduces the problem if possible
  • copy-paste the full content of any console errors emitted by gradlew spotless[Apply/Check] --stacktrace

If you're just submitting a feature request or question, no need for the above.

  • Spotless 6.16.0
  • Gradle 8.0.1
  • MacOS Ventura

    spotless {
        java {
            licenseHeader 'SPDX-License-Identifier: Apache-2.0'
            ignoreErrorForStep('LicenseHeaderStep')
        }
    }
  1. Regex

Our large codebase has some variations in copyright specifically in terms of spaces between comment chars & the license text

So our checkstyle config (not merged, just trying to find a good tool to add this check since we migrated from maven) becomes

<?xml version="1.0"?>
<!DOCTYPE module PUBLIC
        "-//Checkstyle//DTD Checkstyle Configuration 1.3//EN"
        "https://checkstyle.org/dtds/configuration_1_3.dtd">
<module name="Checker">
    <module name="TreeWalker">


        <module name="Regexp">
            <property name="format" value="/\*[ ]{1,}SPDX-License-Identifier: Apache-2\.0"/>
        </module>

        <module name="Regexp">
            <property name="format" value="/\*[ ]{1,}Copyright Contributors to the( ODPi)? Egeria project"/>
        </module>
    </module>
</module>

That seems good enough, so I'm wondering if I can use spotless instead, as it offers more functionality which we may use in future

Obviously we have have a preferred spacing (for auto fix) but want to tolerate additional spaces

I tried without the comments, but then all files fail the check

Interestingly I also wanted to only report warnings, and note that though multiple errors on a project will now be reported, in a multi module build, the build will terminate as that project build fails (unless --continue is used).

Finally, in the example above I'd need two license header steps, but this seems to not be allowed?

@nedtwigg
Copy link
Member

nedtwigg commented Mar 7, 2023

I would read these two resources

To implement what you want, there would still be only one licenseHeader step, but that one step would specify a template which had some kind of regex inside it.

Currently, the license header step doesn't read very much in the existing header. It calculates "this is what the header ought to be", and either it exactly matches or it doesn't.

It does read some things, such as the copyright header. It does this so that it can preserve the existing copyright year and add new years on top.

So it would be possible to add some "softness" into the step, but it would be a new feature. We'd be happy to merge such a feature. I would start by updating the docs for the existing license header step to your dream scenario, and then building to that spec.

@blacelle
Copy link
Contributor

blacelle commented Mar 8, 2023

Given #1607, could this be achieved with 2 licenseHeader steps, each with the proper skipStep('XXX').forFiles('Blah.java') clause? This would argue in favor of steps having an alias (to seggregate between multiple steps with the same com.diffplug.spotless.FormatterStepImpl#getName).

@planetf1
Copy link
Author

@blacelle Thanks for the tip. that's useful for when any of us look again at these checks. For v4 we're likely leave as-is in the interests of time.

@nedtwigg
Copy link
Member

@blacelle I agree that is possible, but it's such an advanced move I'm a bit worried about how to document it, versus adding something like $allow(regex) e.g. $allow(\s*) into the template.

@nedtwigg nedtwigg changed the title licenseHeader - from checkstyle - regex, warn vs error? allow more flexibility in keeping "sloppy" parts of existing license header Mar 13, 2023
@nedtwigg nedtwigg changed the title allow more flexibility in keeping "sloppy" parts of existing license header more flexibility in keeping "sloppy" parts of existing license header Mar 13, 2023
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

3 participants