conda-smithy icon indicating copy to clipboard operation
conda-smithy copied to clipboard

Show conda-smithy version in rendered feedstock files

Open agoodm opened this issue 7 years ago • 10 comments

When rendering a feedstock, the conda-smithy version numbers are not shown in the automatically generated files. In general I think it is a good practice to show version numbers in automatically generated files, and this should be especially helpful for feedstock maintainers since this will help them determine if re-rendering is necessary for certain situations.

agoodm avatar Nov 08 '16 06:11 agoodm

So while this seems like a good idea and I am sympathetic to it, by adding the version to files we can get re-renderings that are nothing more than changing this version. This can be problematic as it results in a lot of churn in the system. See issue ( https://github.com/conda-forge/conda-smithy/issues/168 ) for a more detailed discussion.

jakirkham avatar Nov 08 '16 06:11 jakirkham

Fair enough, I did not fully consider that point. If my understanding is correct, is the precise issue that we need to work around simply just a matter of preventing unnecessary re-renderings (ie when the only changes to the newly rendered files would be the version numbers)? Or is it more complex than that? Either way, I would be interested in helping with finding a working solution to this sort of problem.

agoodm avatar Nov 08 '16 06:11 agoodm

This latest commit adds a "--strict" switch to conda smithy rerender to skip re-rendering any feedstock files in which the only changes are made to comments (which would thus include things like bumping the version number). Then regenerate_feedstock.py would not flag these feedstocks as being in need of an update since the feedstock files would not differ from upstream. Main drawback I can see to this is that you can now have mixed version numbers in the files if some get changed and some don't. This can be worked around but would require something more complicated (like backing up the old feedstock files and then performing the diffs afterwards all at once).

@jakirkham what do you think about this idea?

agoodm avatar Nov 08 '16 18:11 agoodm

ping @conda-forge/core

agoodm avatar Nov 11 '16 02:11 agoodm

this should be especially helpful for feedstock maintainers since this will help them determine if re-rendering is necessary for certain situations

Thanks @agoodm. Would you mind elaborating (just a little 😉 ) on what you percieve those certain situations might be? I'm just curious if there are other ways of hitting them (for example, could that be a structured thing in the git commit history).

pelson avatar Nov 17 '16 19:11 pelson

@pelson So the actual motivation for this PR came when I needed to push a new release in this feedstock. I actually suspected at the time that I might have needed to use conda-smithy but was uncertain if that was necessary since I was unable to determine what version was used to originally generate the feedstock. I do know now that information can be found in the commit history, but in my case it would have done me little good anyway since this won't show for newly created feedstocks (ie, ones that have not had any re-renderings done at least once). Also, upon studying the README and giving myself a modest understanding of what conda-smithy actually does, it seemed far more intuitive for me to check the automatically generated files first for version numbers, which is ultimately what lead me here. Plus, as I have already stated earlier, many projects which automatically generate boilerplate code or configuration files will often include version numbers at the top, so I thought it would be good practice.

agoodm avatar Nov 17 '16 20:11 agoodm

... that information can be found in the commit history, but ... this won't show for newly created feedstocks ...

This is a really good point. We should include the conda-smithy version during feedstock creation in a commit somehow. I see two main options. Both are very doable (if you have interest). 😉

  1. Change these lines to either use --commit auto or some message with the version.
  2. Change the first commit message to include the version.

IMHO 2 is slightly better, but either is fine and should help. 😄

jakirkham avatar Nov 17 '16 22:11 jakirkham

I also like option 2. In that case, what is your current stance on having version numbers in the generated files? If it is something that you guys don't feel comfortable with at the moment, then it might just be better to handle the commit message change in a new PR since that's a very simple thing to change.

agoodm avatar Nov 17 '16 23:11 agoodm

Making the versioning explicit is a good idea and often requested (had a similar conversation with a coworker on this not that long ago), but trying to figure out the right way to do that is definitely tricky.

Getting the version in the first commit message seems like an easy win in the interim.

jakirkham avatar Nov 18 '16 01:11 jakirkham

Ok, very well then. I will go ahead and make another PR shortly. In the meantime, do feel free to test out some of the changes made in this one (ie, skipping rerendering for changes which only include commented lines) since I do think that it might have some promise though I don't fully know how useful it would actually be in practice.

agoodm avatar Nov 18 '16 02:11 agoodm