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

'Update' file headers: license notices, \file and other minor changes #1793

Merged
merged 13 commits into from
Oct 26, 2021

Conversation

rodolforg
Copy link
Contributor

@rodolforg rodolforg commented Oct 19, 2020

The fixes/adaptations in source code file headers (check synfig-core/src/modules/lyr_std/freetime.cpp for example):

  • 'Update' license notice as for that suggested by FSF
  • Remove $Id$ – useful in CVS/SVN era, not used since project migrated to Git
  • Fix incorrect filenames in \file doxygen command – it was added in some files, specially on ETL folder
  • Remove empty === N O T E S ========= sections of file headers.

The license notices were changed to follow the GNU suggestion of how to do it:
http://www.gnu.org/licenses/gpl-howto.html (section "The license notices").

I kept the indentation of each original header, although I don't see why use tabular here.

This file is part of Synfig.

Synfig is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 2 of the License, or
(at your option) any later version.

Synfig is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with Synfig.  If not, see <https://www.gnu.org/licenses/>.

@rodolforg rodolforg marked this pull request as draft October 19, 2020 21:49
@rodolforg rodolforg marked this pull request as ready for review October 19, 2020 21:58
Copy link
Member

@morevnaproject morevnaproject left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have found small issue with repeated word. Please check my comment. ^__^

ETL/ETL/_angle.h Show resolved Hide resolved
@ghost
Copy link

ghost commented Oct 23, 2020

Congratulations 🎉. DeepCode analyzed your code in 18.71 seconds and we found no issues. Enjoy a moment of no bugs ☀️.

👉 View analysis in DeepCode’s Dashboard | Configure the bot

@rodolforg
Copy link
Contributor Author

rodolforg commented Oct 23, 2020

I have plans to some other minor changes in files headers.
Should I place them in other PR or include them into a single PR?

  • Remove $Id$ line - unused for the last years, when started to use git
  • Sort the #include in a more organized ways (like @Keyikedalube and @blackwarthog do)
    • alphabetically
    • separated by groups:
      • (equivalent .h)
      • C++ standards
      • Glibmm/Gtkmm
      • ETL
      • Synfig
      • SynfigStudio
  • I'd like to change the copyright list to reduce its verbose info in some case to a similar way Godot Engine does and Google proposes:
**	\legal
**	Copyright (c) 2002-2005 Robert B. Quattlebaum Jr., Adrian Bentley
**	Copyright (c) 2005-2020 The Synfig authors
// or
**	Copyright (c) 2005-2020 Synfig contributors (cf. AUTHORS.md)

@ice0
Copy link
Collaborator

ice0 commented Oct 24, 2020

I think all header changes better to make in one PR, #include reorder in another.

@morevnaproject
Copy link
Member

The PR is looks good now! I think it is ready to merge! ^__^

morevnaproject
morevnaproject previously approved these changes Oct 24, 2020
Copy link
Member

@morevnaproject morevnaproject 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!

@morevnaproject
Copy link
Member

  • Remove $Id$ line - unused for the last years, when started to use git

  • I'd like to change the copyright list to reduce its verbose info in some case to a similar way Godot Engine does and Google proposes:

I agree, those header changes are good to go with single PR, and includes in separate one. ^_^

@rodolforg
Copy link
Contributor Author

@morevnaproject @ice0

Any decision or other proposal for Copyright statements?

  1. Replace non-original synfig authors with ** Copyright (c) 2005-2020 The Synfig authors
  2. Replace non-original synfig authors with ** Copyright (c) 2005-2020 Synfig contributors (cf. AUTHORS.md)
  3. Don't replace, but append ** Copyright (c) 2005-2020 The Synfig authors
  4. Don't replace, but append ** Copyright (c) 2005-2020 Synfig contributors (cf. AUTHORS.md)
  5. Don't touch it at all

The AUTHORS or AUTHORS.md file may begin with, as for Google suggestion,

# This is the list of [Project Name]'s significant contributors.
#
# This does not necessarily list everyone who has contributed code,
# especially since many employees of one corporation may be contributing.
# To see the full list of contributors, see the revision history in
# source control.

@rodolforg rodolforg force-pushed the some-file-header-fixes branch 2 times, most recently from d60015e to bfef733 Compare October 25, 2020 02:50
@rodolforg
Copy link
Contributor Author

I (still?) didn't remove those lines from .sgml files in synfig-docs folder.

<!-- retain these comments for translator revision tracking -->
<!-- $Id$ -->

Maybe the entire folder should be deleted anyway.

@rodolforg rodolforg marked this pull request as draft November 5, 2020 22:36
@rodolforg
Copy link
Contributor Author

I still will fix the \file and \brief doxygen commands "soon".

But I don't know what to do with copyright statements as I told here #1793 (comment) – if we'll do something about it. I think we should go for options 3 or 4, because probably we would need the explicit permission of the listed authors to remove their explicit names from there (some licenses require to retain the copyright notices – I'm not sure about GPLv2).
I allow to remove all my copyright statements on synfig project files :P

@rodolforg rodolforg force-pushed the some-file-header-fixes branch 2 times, most recently from 9a444eb to e31de4c Compare November 7, 2020 02:33
@rodolforg
Copy link
Contributor Author

rodolforg commented Nov 7, 2020

The fixes/adaptations until now (check synfig-core/src/synfig/cairo_operators.cpp for example):

  • 'Update' license notice as for that suggested by FSF
  • Remove $Id$ – useful in CVS/SVN era, not used since project migrated to Git
  • Fix incorrect filenames in \file doxygen command – it was added in some files, specially on ETL folder
  • Fix contents of \brief, another doxygen command: most of cases used the Template brief file description, copied from… template file :P
  • Remove empty === N O T E S ========= sections of file headers.
  • Adapt copyright notice to simplify it and trust the copyright to git repository history, as a lot of opensource projects currently do (as Google ones and Godot Engine, for example)

The last one I need opinions from you @morevnaproject @ice0 (please see my previous posts).
The fourth item is a bit annoying/tedious to do :P

@rodolforg
Copy link
Contributor Author

As I have no decision about copyright notices/authors, I'll stop this PR here.

@ice0
Copy link
Collaborator

ice0 commented Oct 19, 2021

@morevnaproject @rodolforg
A lot of work has been done here so I think better to merge it as-is.

@rodolforg
Copy link
Contributor Author

OK

@morevnaproject
Copy link
Member

A lot of work has been done here so I think better to merge it as-is.

Absolutely agree ^__^

It was useful and used in CVS and SVN ages, not in git.

Those 'old' versioning systems replace the $Id$ with
the current version id on checkout, but git doesn't do it.
It was useful and used in CVS and SVN ages, not in git.

Those 'old' versioning systems replace the $Id$ with
the current version id on checkout, but git doesn't do it.
It was useful and used in CVS and SVN ages, not in git.

Those 'old' versioning systems replace the $Id$ with
the current version id on checkout, but git doesn't do it.
It was useful and used in CVS and SVN ages, not in git.

Those 'old' versioning systems replace the $Id$ with
the current version id on checkout, but git doesn't do it.
and some respective \brief (error due to copy and paste)

These are special commands for Doxygen documentation generator.
and some respective \brief (error due to copy and paste)

These are special commands for Doxygen documentation generator.
and set some \brief (already existent, but not annotated as so).
These are special commands for Doxygen documentation generator.
It should be a file contents description, but they use the
template file description
or replace it with \note doxygen command when not empty
@rodolforg
Copy link
Contributor Author

Rebased then ;)

@ice0 ice0 merged commit 333182e into synfig:master Oct 26, 2021
@ice0
Copy link
Collaborator

ice0 commented Oct 26, 2021

Merged. Thank you!

@rodolforg rodolforg deleted the some-file-header-fixes branch October 26, 2021 11:49
@rodolforg
Copy link
Contributor Author

Regarding the copyright statement, should we/I do something? XD

Any decision or other proposal for Copyright statements?

1. Replace non-original synfig authors with `**	Copyright (c) 2005-2020 The Synfig authors`

2. Replace non-original synfig authors with  `**	Copyright (c) 2005-2020 Synfig contributors (cf. AUTHORS.md)`

3. Don't replace, but append  `**	Copyright (c) 2005-2020 The Synfig authors`

4. Don't replace, but append  `**	Copyright (c) 2005-2020 Synfig contributors (cf. AUTHORS.md)`

5. Don't touch it at all

@morevnaproject
Copy link
Member

I suggest to go with 4.
Opinions? ^__^

@rodolforg
Copy link
Contributor Author

I'm okay with it.
But my cleanup-obsession wants to contact everyone to ask if we can remove their explicit name of per-file-copyright statement lol

@morevnaproject
Copy link
Member

I think there is no harm if we leave their names in file.

But I do not mind if you will remove my name. ^__^

@rodolforg
Copy link
Contributor Author

I don't care about mine too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants