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

Ignore files marked with @generated marker #1689

Merged
merged 8 commits into from
Jun 28, 2021

Conversation

stepancheg
Copy link
Contributor

@stepancheg stepancheg commented Jun 23, 2021

@generated marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:

  • these files do not need to be reformatted,
  • diffs in these files are less important,
  • and linters should not be invoked on these files.

This PR proposes builtin support for @generated marker (and
@not-generated marker to mark file as not generated when it
contains @generated marker, like README.md).

I have not found a standard for a generated file marker, but:

Super-linter supports regex includes and excludes, but they are
harder to maintain (each repository needs to be configured) than
patching the tools which generate the files.

My personal story is that I maintain rust-protobuf crate, which
started emitting @generated markers six years ago
after a request of a Phabricator user.

Test Plan:

Create a test file test.sh:

echo $a

Run:

docker run -e RUN_LOCAL=true -v $HOME/tmp/g:/tmp/lint super-linter-test

Result is:

In /tmp/lint/test.sh line 1:
echo $a
^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
     ^-- SC2154: a is referenced but not assigned.
     ^-- SC2086: Double quote to prevent globbing and word splitting.
...
2021-06-22 23:46:16 [ERROR]   ERRORS FOUND in BASH:[1]

Now add @generated to the file and run again:

2021-06-22 23:47:13 [NOTICE]   All file(s) linted successfully with no errors detected

Additionally, add @not-generated in addition to @generated, and
linter error pops up again.

Readiness Checklist

Author/Contributor

  • If documentation is needed for this change, has that been included in this pull request

Reviewing Maintainer

  • Label as breaking if this is a large fundamental change
  • Label as either automation, bug, documentation, enhancement, infrastructure, or performance

`@generated` marker is used by certain tools to understand that the
file is generated, so it should be treated differently than a file
written by a human:
* these files do not need to be reformatted,
* diffs in these files are less important,
* and linters should not be invoked on these files.

This PR proposes builtin support for `@generated` marker (and
`@not-generated` marker to mark file as not generated when it
contains `@generated` marker, like `README.md`).

I have not found a standard for a generated file marker, but:
* Facebook [uses `@generated` marker](https://tinyurl.com/fb-generated)
* Phabricator tool which was spawned from Facebook internal tool
  [also understands `@generated` marker](https://git.io/JnVHa)
* Cargo inserts `@generated` marker into [generated Cargo.lock files](https://git.io/JnVHP)

Super-linter supports regex includes and excludes, but they are
harder to maintain (each repository needs to be configured) than
patching the tools which generate the files.

My personal story is that I maintain rust-protobuf crate, which
started emitting `@generated` markers [six years ago](https://git.io/JnV5h)
after a request of a Phabricator user.

Test Plan:

Create a test file `test.sh`:

```
echo $a
```

Run:

```
docker run -e RUN_LOCAL=true -v $HOME/tmp/g:/tmp/lint super-linter-test
```

Result is:

```
In /tmp/lint/test.sh line 1:
echo $a
^-- SC2148: Tips depend on target shell and yours is unknown. Add a shebang or a 'shell' directive.
     ^-- SC2154: a is referenced but not assigned.
     ^-- SC2086: Double quote to prevent globbing and word splitting.
...
2021-06-22 23:46:16 [ERROR]   ERRORS FOUND in BASH:[1]
```

Now add `@generated` to the file and run again:

```
2021-06-22 23:47:13 [NOTICE]   All file(s) linted successfully with no errors detected
```

Additionally, add `@not-generated` in addition to `@generated`, and
linter error pops up again.
@stepancheg
Copy link
Contributor Author

Perhaps this need to be labeled as "enhancement" but GitHub doesn't allow me to do that.

@ferrarimarco ferrarimarco added documentation Improvements or additions to documentation enhancement New feature or request labels Jun 23, 2021
@ferrarimarco
Copy link
Collaborator

@stepancheg thanks for this PR!

This looks interesting. I added a few comments

README.md Outdated Show resolved Hide resolved
lib/functions/buildFileList.sh Outdated Show resolved Hide resolved
@stepancheg
Copy link
Contributor Author

Amended the PR:

  • use IGNORE_GENERATED_FILES variable, false by default
  • more documentation

README.md Outdated Show resolved Hide resolved
@ferrarimarco
Copy link
Collaborator

Nice work!

@stepancheg
Copy link
Contributor Author

Updated the readme.

@stepancheg
Copy link
Contributor Author

I have created a little website to promote @generated marker: https://generated.at/

@ferrarimarco
Copy link
Collaborator

@admiralAwkbar want to have a look as well?

Copy link
Collaborator

@admiralAwkbar admiralAwkbar left a comment

Choose a reason for hiding this comment

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

This looks really good!
I need to get a fix in for the phar that's breaking all current builds... Shoudl have that done today and we can merge master into this branch and see where it goes!

Thanks for the cool work!

admiralAwkbar
admiralAwkbar previously approved these changes Jun 24, 2021
Copy link
Collaborator

@admiralAwkbar admiralAwkbar left a comment

Choose a reason for hiding this comment

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

looks good

@admiralAwkbar
Copy link
Collaborator

@ferrarimarco all good to you?

@admiralAwkbar
Copy link
Collaborator

So the fail was from Raku as when you run the version command it returns non ascii chars... its super dumb... So I had to update the command to pull those out and stop making the makefile angry...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants