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

Modernized Readme file #3745

Merged
merged 9 commits into from
Jun 5, 2020
Merged

Modernized Readme file #3745

merged 9 commits into from
Jun 5, 2020

Conversation

jngrad
Copy link
Member

@jngrad jngrad commented May 31, 2020

Follow-up to the 2020-05-26 ESPResSo meeting

Description of changes:

  • explain how to get released versions
  • invite users to subscribe to the mailing list
  • give clear instructions on how to cite ESPResSo
  • for contributors, briefly explain our release workflow

Invite users to join the community by subscribing to the mailing
list to keep up-to-date with releases and give feedback in surveys.
For new contributors, give a simplified overview of the release
workflow and explain how to find the live release notes.
@jngrad jngrad added this to the Espresso 4.1.3 milestone May 31, 2020
@jngrad jngrad requested a review from RudolfWeeber May 31, 2020 17:07
@codecov
Copy link

codecov bot commented May 31, 2020

Codecov Report

Merging #3745 into python will increase coverage by 0%.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           python   #3745   +/-   ##
======================================
  Coverage      89%     89%           
======================================
  Files         552     552           
  Lines       24532   24532           
======================================
+ Hits        21853   21861    +8     
+ Misses       2679    2671    -8     
Impacted Files Coverage Δ
src/core/particle_data.cpp 96% <0%> (+<1%) ⬆️
src/core/electrostatics_magnetostatics/p3m.cpp 87% <0%> (+<1%) ⬆️
src/core/polymer.cpp 98% <0%> (+6%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d509f7...448c048. Read the comment docs.

fweik
fweik previously approved these changes Jun 2, 2020
@fweik
Copy link
Contributor

fweik commented Jun 2, 2020

This also reflects my understanding of the API guarantees that we want to make.

Max-Planck-Institute for Polymer Research, Theory Group


Copyright (C) 2002-2010 Max-Planck-Institute for Polymer Research, Theory Group
Copy link
Member

Choose a reason for hiding this comment

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

I think we should just refer to the (non-existing) License file that could then also serve as the header template.

Copy link
Member Author

Choose a reason for hiding this comment

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

I would be fine with shortening this section of the Readme to the following text, which links to the license and acknowledges both the MPI and ESPResSo organizations:

Copyright (C) 2010-2019 The ESPResSo project
Copyright (C) 2002-2010 Max-Planck-Institute for Polymer Research, Theory Group
ESPResSo 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 3 of the License, or (at your option)
any later version. A copy of the license is provided in [COPYING](COPYING).

The issue with providing instead a link to the header template is that the MPI line is missing.

Copy link
Member

Choose a reason for hiding this comment

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

Why is the MPI part missing in the header template? I think we should move the header template to LICENSE in the root and add the MPI part.

Copy link
Member Author

Choose a reason for hiding this comment

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

The license header can only contain years where the file received non-trivial contributions, therefore the MPI cannot appear for files created from scratch after 2010. Having this header template puts us in a grey area, since source files committed without a license header in the first commit where they appear are automatically licensed in a way that depends on the country(ies) of origin of the contributor, if I recall correctly. Adding the GPL after the fact during releases is not really permitted without the agreement of all people who committed non-trivial things in that file since its creation. That's why Google and Fedora have annoying bots that reject your contributions until you sign a legal document that allows them to relicense your contributions as GNU GPL if you forgot to put it in the first commit.

If I'm not mistaken, COPYING is the standard place to put the license in GNU GPL projects, while LICENSE is the standard place to put the license in Apache projects.

@RudolfWeeber
Copy link
Contributor

Can the mailing list info be moved up? I don't think most people will read that far. If possible, the mailing list info should be immediately after the brief description of ES's purpose.

@jngrad
Copy link
Member Author

jngrad commented Jun 2, 2020

@RudolfWeeber Placing the mailing list inside the Installation section of the Readme felt natural to me, as new users would typically look first for the installation instructions. By putting it after the ES description, i.e. before the documentation and installation instructions, I'm afraid new users might just skip it. I can move up the mailing list a little bit, between the installation and citation section above, but if you insist I'll place it after the ES description, as you proposed.

@RudolfWeeber
Copy link
Contributor

JN, please proceed as you see fit with this PR.

@jngrad jngrad added the automerge Merge with kodiak label Jun 5, 2020
@kodiakhq kodiakhq bot merged commit eb54eda into espressomd:python Jun 5, 2020
@jngrad jngrad deleted the rewrite-readme branch January 18, 2022 12:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants