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

[FEATURE] Configure spotless to handle newline linting in lieu of linelint #110

Closed
dbwiddis opened this issue Aug 29, 2022 · 2 comments · Fixed by #193 or #197
Closed

[FEATURE] Configure spotless to handle newline linting in lieu of linelint #110

dbwiddis opened this issue Aug 29, 2022 · 2 comments · Fixed by #193 or #197
Assignees
Labels
discuss enhancement New feature or request

Comments

@dbwiddis
Copy link
Member

dbwiddis commented Aug 29, 2022

Is your feature request related to a problem?

Issue #46 requested to add linelint to this repo matching OS #2857. I commented that it could have been done with spotless, but there was no further discussion and linelint was implemented as a GitHub Action.

This presents a frustrating situation for users, as linelint does not automatically fix errors. (The actual program does but this is not helpful from a GHA.). There is no real way to proactively lint files from the command line, or even check them, prior to submitting a PR, seeing a failing lint check, and then manually fixing the affected files.

I would like to re-raise the issue of using a single linter (spotless) which not only detects but auto-fixes these issues.

Contrary to the comments on OS #2857, it can handle all sorts of files and is not limited to just .java files. See for example here where .gradle and .md (and other) files are included:

  format 'misc', {
    // define the files to apply `misc` to
    target '*.gradle', '*.md', '.gitignore'

    // define the steps to apply to those files
    trimTrailingWhitespace()
    indentWithTabs() // or spaces. Takes an integer argument if you don't like 4
    endWithNewline()
  }

What solution would you like?

Remove linelint and configure spotless as above (but with spaces, not tabs).

Consider making the same change in OpenSearch for the same reasons and with a much wider impact.

What alternatives have you considered?

Building linelint on my local machine so I can take advantage of its auto-fix features. I already tried to install with homebrew but it's not set up there and the effort vs. time saved isn't worth building it myself on multiple machines; or even if it was, that would only help me.

Do you have any additional context?

I plan on adding more .svg files documenting designs, and they don't include newlines by default. I don't want to manually edit .svg files. I don't want to upload .svg files and get yelled at by failed GHA. I don't want to keep adding exclusions to linelint.yml. I just want to run ./gradlew spotlessApply which I already do, and not have to think about end of file newlines ever again.

@dbwiddis dbwiddis added enhancement New feature or request untriaged labels Aug 29, 2022
@owaiskazi19 owaiskazi19 added the good first issue Good for newcomers label Aug 31, 2022
@owaiskazi19
Copy link
Member

owaiskazi19 commented Aug 31, 2022

@peternied do we have any advantage of using the current line linter as compared to Spotless linelint? If not, then we are good to go with Spotless :)

@dbwiddis
Copy link
Member Author

dbwiddis commented Sep 6, 2022

https://github.com/opensearch-project/opensearch-sdk-java/runs/8199437661?check_suite_focus=true

Note for future me to reduce that 11 minutes to 2 minutes until this is changed, ed has a nice trick:

$ ed -s openapi.json <<< w
newline appended

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss enhancement New feature or request
Projects
None yet
2 participants