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

Manage license headers with headroom instead of licensure #1084

Merged
merged 13 commits into from Apr 30, 2020

Conversation

mheinzel
Copy link
Contributor

For the most parts, it's a drop in replacement, but it has some advantages.

It's a Haskell tool, so it can be installed (at exactly the version we want) through Stack, the same way as Ormolu.

Also, it knows that we want our Haskell headers after the pragmas (in line with Ormolu's style).
This way:

  • Ormolu will already be happy after adding a header
  • Headroom is idempotent even in conjunction with Ormolu, making it unnecessary to detect existing headers with grep
  • Headroom will correctly update headers if they differ from the template (e.g. updating the year)

Thanks a lot to @vaclavsvejcar for adding the features required to make this work!

Copy link
Member

@akshaymankar akshaymankar 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! Does this mean we can also add a check in the PR pipeline for license headers?

@mheinzel
Copy link
Contributor Author

Yes, we can add it right now by running Headroom and then checking if any files changed. Or even by checking for 0 modified in the output. 😛

There also is an issue for a built-in command, similar to Ormolu's --mode check. If that gets implemented, it would be less hacky.

@akshaymankar
Copy link
Member

Yes, we can add it right now by running Headroom and then checking if any files changed. Or even by checking for 0 modified in the output. stuck_out_tongue

There also is an issue for a built-in command, similar to Ormolu's --mode check. If that gets implemented, it would be less hacky.

Let's do it then :)

@vaclavsvejcar
Copy link

Glad to hear Headroom works for you 👍 Feel free to create new issue if you'll have ideas for new features. I also expect that the mentioned built-in command could be part of one of the next releases.

@mheinzel
Copy link
Contributor Author

Then I'll add a CI check once the command is implemented in Headroom.

@mheinzel mheinzel merged commit b2de9c7 into develop Apr 30, 2020
@mheinzel mheinzel deleted the licenses-headroom-onto-licensure branch April 30, 2020 12:23
@akshaymankar akshaymankar mentioned this pull request May 7, 2020
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.

None yet

3 participants