vaclavsvejcar / headroom

Šī¸ Manager for license headers in source code files.
https://doc.norcane.com/headroom/latest/
BSD 3-Clause "New" or "Revised" License
48 stars 3 forks source link

Allow template-path to be URL #66

Closed vrom911 closed 3 years ago

vrom911 commented 3 years ago

I wonder if you think it could be possible to have something like this. My case is that lots of repositories have the same template we are using through our organization. It would be nice if we could have one template in the dedicated repo and in each particular project's .headroom.yaml we could just provide a link in there, instead of adding the templates folder in each project.

This is just an idea, I want to see what do you think about it and what options there are 🙂

vaclavsvejcar commented 3 years ago

This is pretty cool idea, thanks for suggesting it! 🙂 Thinking about the implementation, adding command line argument or YAML config option for referencing single URL template would be quite straightforward, but I'm not sure how to do something like referencing folder of templates (like you can do currently on local filesystem). The latter would likely require to add some more sophisticated support for template repositories, etc. What do you think? Or would the option to specify URL templates one by one be sufficient for now?

vrom911 commented 3 years ago

Yes, I think this would be awesome to have with separate file URLs at the moment. I completely agree that it is too complicated with folder, but the separate file URL in config/CLI would be fantastic for general use-case!

vaclavsvejcar commented 3 years ago

Awesome, let's make this happen in upcoming v0.4.1.0 🙂

vaclavsvejcar commented 3 years ago

So I started to think about this one and at least for now, there are some open questions that will need to be decided before actually implementing this. I'm mainly just thinking aloud here but maybe @vrom911 you'll also have some ideas about it 🙂

First, for each template file, Headroom needs to know for which type of source code file this template is and what type of template engine to use. In locally stored templates this is specified in template name as <FILE_TYPE>.<TEMPLATE_TYPE_EXT>, such as folder/haskell.mustache. Question is how to do this for URL located templates. I guess I can just require that the URL contains this information as well, so Mustache template for Haskell would be available for example as https://example.com/path/haskell.mustache, but I'm not sure if it isn't too much restricting. Otherwise it would be necessary to specify this info together with the URL in cmdline parameter or in YAML config file.

Second, I'll really need to rethink how to handle precedence of templates if you define same template from multiple sources. Including this change, Headroom will allow to use 3 different sources of templates. First, built-in templates available under --builtin-templates=<LICENSE> cmdline argument, second are templates stored in template folder and third will be template fetched from URL. When it happens that let's say two Mustache templates for Haskell will be defined at the same time, Headroom will somehow need to decide how to handle this. One option is to just reject such state, but it prevent use cases when you want to use let's say built-in templates and override only single one by your custom template either from local file or URL. Another option is to define override strategy, but I don't want to decide this for user. I'd rather make this configurable with default configuration that would allow overriding in direction like built-in templates -> local templates -> URL templates and if that doesn't fit the end user, she/he can just change it using some configuration such as something like:

template-priority:
  built-in: 1
  local: 2
  url: 3

What do you think about these points? Would such solution also match your needs?

EDIT: Moving this issue to v0.4.2.0 milestone as I need to do another minor release first.

chshersh commented 3 years ago

Hey @vaclavsvejcar! Your proposal looks good to me 👍đŸģ Can't wait to use this feature in action 🙂

I have slightly different thoughts about the design. I'm not sure that the benefit of user-specified flexibility worth the implementation cost. I think the built-in functionality should have the lowest priority. You can't change its behaviour and it's always available to you. Also, I think that the local file priority should be higher than the url one in case you want to download the template as a cache and avoid downloading. Basically, the reasoning is "the closer the source to you and the more control over it you have, the higher priority it should have".

As a side note, I'm thinking about native support for Dhall in headroom. The Dhall support definitely deserves a separate issue and thorough consideration of all trade-offs. But I'm mentioning it here because Dhall natively supports referring to resources by URL. And with Dhall we can reduce the .headroom.yaml config itself in all our repositories to 2 lines of reusable Dhall function call, which would be ideal for our use case.

These are just a few thoughts from my side, and I'm not insisting on any particular implementation 🙂 headroom already helps us a lot in maintaining module headers! 🔝

vaclavsvejcar commented 3 years ago

Hello @chshersh, thank you for yout thoughts.

I have slightly different thoughts about the design. I'm not sure that the benefit of user-specified flexibility worth the implementation cost. I think the built-in functionality should have the lowest priority. You can't change its behaviour and it's always available to you. Also, I think that the local file priority should be higher than the url one in case you want to download the template as a cache and avoid downloading. Basically, the reasoning is "the closer the source to you and the more control over it you have, the higher priority it should have".

Good point. Regarding the ability to configure the behaviour - I guess I'll just skip this part and implement it if really needed in future. And regarding the priority - I like the idea that "the closer the source to you and the more control over it you have, the higher priority it should have", so let's have it that way 🙂

As a side note, I'm thinking about native support for Dhall in headroom. The Dhall support definitely deserves a separate issue and thorough consideration of all trade-offs. But I'm mentioning it here because Dhall natively supports referring to resources by URL. And with Dhall we can reduce the .headroom.yaml config itself in all our repositories to 2 lines of reusable Dhall function call, which would be ideal for our use case.

I know about Dhall, but have zero experience with it. When I was writing Headroom, I choose YAML because just everyone knows it. That's also the reason why I probably wouldn't drop YAML support, but eventually just add Dhall as second way how to configure Headroom. Anyway, I'll check it out and I created follow-up ticket so we can discuss it there 🙂

chshersh commented 3 years ago

@vaclavsvejcar I totally support your idea of not dropping the first choice of plain config option and providing Dhall only as an additional bonus. I have some experience with Dhall: we generate HLint warnings for relude and we use it at work. If that's okay with you, I can provide some thoughts on how this possibly could look and work in headroom.

vaclavsvejcar commented 3 years ago

@vaclavsvejcar I totally support your idea of not dropping the first choice of plain config option and providing Dhall only as an additional bonus. I have some experience with Dhall: we generate HLint warnings for relude and we use it at work. If that's okay with you, I can provide some thoughts on how this possibly could look and work in headroom.

@chshersh That would be awesome, I'd appreciate any experience with Dhall 🙂 I'll also give it a try right after I'll finish this one with URL-based templates.

vaclavsvejcar commented 3 years ago

Hello @vrom911 @chshersh, I implemented this change and it's merged in master, so you can expect in next Headroom release. I still need to do some testing to make sure it's production ready, but you can already test it and I'll be grateful for any feedback 🙂

In order to use it, just place the HTTP/HTTPS address where you normally put the local template path, i.e. either as command line option -t|--template-path https://foo.bar/haskell.mustache or as YAML configuration key

template-paths:
  - some/local/rust.mustache
  - https://foo.bar/haskell.mustache
vaclavsvejcar commented 3 years ago

@chshersh @vrom911 Hello, not sure if you got previous notification, just please when you'll have some time to take a quick look, let me know if current implementation suits you so I can eventually release it. Thanks a lot 🙂

chshersh commented 3 years ago

Hi @vaclavsvejcar! Indeed, the previous notification somehow was unnoticed. We've tested the latest version of headroom and introduced our templates in our org repository:

We tested the configuration and everything is working as expected 👍đŸģ Thanks a lot for implementing this feature, it's very helpful!

vaclavsvejcar commented 3 years ago

@chshersh Awesome, happy to hear that! I'll then publish this in few days as part of the v0.4.2.0 release.

vaclavsvejcar commented 3 years ago

Released as part of v0.4.2.0.