Skip to content

Draft: add possibility to append license and copyright statements independently

Vava Gligorov requested to merge vava-add-license into master

@clemenci @cburr Here is my attempt to factorize out adding license and copyright statements to files. Some deliberate choices

  1. For porcelain printout I merge files which are missing copyright and those missing licenses
  2. For non-porcelain printout I separately list the two
  3. When adding statements to files which lack one or the other, I treat the two possibilities completely separately. This is robust because I can insert each statement standalone using the same logic as this script already used, and I can handle corner-cases where someone added a copyright or license statement by hand which is not formatted in the same way as copyright and license statements added by this script. The downside is that files which already had a license or copyright statement end up looking different from files which had neither. For the same reason files which begin with either a copyright or license statement, but not both, will be modified by this script in a way which will make them look different to the vast majority of files already in the codebase. However it seemed to me that any other logic was both more fragile and internally inconsistent. If we wanted to do better I would propose that the functions which check for the copyright and/or license check for the exact form added by this script, rather than simply the words copyright, license, or licence. The downside of this approach is that you can end up (admittedly rarely) with contradictory copyright and license notices in some files which already had notices that did not conform to the standard. Feedback is very welcome.
  4. For similar reasons as the previous item I did not yet work on updating the year range as you suggested @clemenci but if we can resolve how we'd like to proceed on that point I'm happy to add this functionality too.

@cagapopo @raaij One thing about using this in Allen is that the CI would need to have the license-fn argument added since Allen chooses to place its license file in LICENSE and not COPYING which is the default

I've tested the cases I could think of locally but help on how to test the production cases would be much appreciated.

Addresses #82

fyi @rmatev

Edited by Vava Gligorov

Merge request reports