search

IUPAC-InChI / InChI

Main InChI repository
https://iupac-inchi.github.io/InChI-Web-Demo/
MIT License
69 stars 9 forks source link

replace all instances of old license text. Fixes #33 #34

Closed giallu closed 3 months ago

giallu commented 4 months ago

As agreed in #33 I replaced all the source code instances left of the old license test with the MIT one and removed the PDF file.

As you can see, I actually just removed the license text from readme files, I think it made no sense to also have it there since it is not code.

Few references to the old license can still be found in docs, but it was not clear to me how to fix since there are multiple formats and I am not sure how you build one from the other. See for instance https://github.com/IUPAC-InChI/InChI/blob/7cb718ffe999945fddfe43acb0cd4920ad302f67/INCHI-1-DOC/FAQ/html/inchi-faq.html#L545-L561 and beyond

My recommendation for the future is to consider removing the license from all files (except those with a different one of course) or at least remove the first lines in all source files so they don't need to be updated. Eg: line 9 should be the first one here:

https://github.com/IUPAC-InChI/InChI/blob/7cb718ffe999945fddfe43acb0cd4920ad302f67/INCHI-1-SRC/INCHI_BASE/src/ichicano.h#L1-L9

djb-rwth commented 4 months ago

Hi @giallu, Thank you for this pull request. I assigned @kiddr and @gblanke02 to approve it, with whom you have already had an extensive discussion in Issue #33.

djb-rwth commented 4 months ago

Hi again @giallu, Forgot to mention one important thing -- all commits should now be submitted to our development branch rwth which will be merged with the main branch later on (and consequently labelled as a new InChI version v.1.07.1). I think this will not affect your commits, but please do check if they fit into rwth branch files.

giallu commented 4 months ago

@djb-rwth thanks for the info about the development branch, I will try to remember that. Please note, you can make this automatic by changing the default branch in the project's settings; if you put rwth there all future's PR will be against that branch

giallu commented 4 months ago

I can't comment on the the code amends (which mostly look to be spaces)

Ahhhh sorry, that must be vscode that triggered some reformat on saving. I will push an update that avoids any change not related to licensing

djb-rwth commented 4 months ago

I can't comment on the the code amends (which mostly look to be spaces)

Ahhhh sorry, that must be vscode that triggered some reformat on saving. I will push an update that avoids any change not related to licensing

Hi @giallu, Please do not worry about chaotic whitespaces as they exist all over the code; the corrected ones were also fine (and looked nicer).

giallu commented 4 months ago

Hi @giallu, Please do not worry about chaotic whitespaces as they exist all over the code; the corrected ones were also fine (and looked nicer).

Yeah, it looked nicer but from personal experience it's better to avoid mixing different topics in the same commit/PR

If desired, we can mass reformat the sources with some tool like that in another PR

djb-rwth commented 3 months ago

Approved by Gerd Blanke as well. Merging.