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

[Docs] Add note on module development about the copyrights #26812

Merged
merged 4 commits into from Jul 19, 2017

Conversation

ryansb
Copy link
Contributor

@ryansb ryansb commented Jul 14, 2017

SUMMARY

Coding guidelines has this already, but it's not in the docsite anywhere I could fine.

ISSUE TYPE
  • Docs Pull Request
COMPONENT NAME

docs

ANSIBLE VERSION

ADDITIONAL INFORMATION

@ansibot ansibot added affects_2.4 This issue/PR affects Ansible v2.4 docs_pull_request needs_triage Needs a first human triage before being processed. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Jul 14, 2017
@@ -125,6 +125,15 @@ Include it in your module file like this:
# ... snip ...
'''

.. note::
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this needs moving earlier in the file and not as a .. note::
This document should be in the same order as the module file itself

@s-hertel s-hertel removed the needs_triage Needs a first human triage before being processed. label Jul 14, 2017
@ryansb ryansb force-pushed the copyright-info-dev-guide branch 2 times, most recently from 749549a to 01f57d1 Compare July 17, 2017 13:14

- The copyright/license declaration should be ONLY two lines, not the full
GPL prefix. If you notice a module with the full prefix, feel free to
switch it to the one-line declaration instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

Break this into two pieces. Copyright and license should be separate entries. Rephrase the second paragraph so it only refers to the license declaration. Can add a note as to why (not needed + modules are transferred over the wire so extra comments are inefficient.)

@@ -116,7 +116,24 @@ Include it in your module file like this:
.. code-block:: python

#!/usr/bin/python
# Copyright header....
# Copyright © 2017 [REPLACE THIS]
Copy link
Contributor

Choose a reason for hiding this comment

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

Invalid python :-) I'd use (C) here. Alternative is to use:

# -*- coding: utf-8 -*-

as the second line in the file.

line. This is especially true of rewrites, but original authorship
copyright messages should be preserved. If applicable, contributors may
have their rights assigned to their employer for example: "Copyright ©
2017 Red Hat, Inc."
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of talking about copyright assignment, maybe just talk about copyright holder? Or maybe we don't want to talk about rights and assignment and such at all? We decided that we're going to start using "Copyright (C) 2017 Ansible Project", for instance. Ansible Project isn't a legal entity so it seems to not be about copyright holder...

This matches what's in CODING_GUIDELINES.md as of July 2017

The first 3 lines of every module should look the same. The first line is the
shebang, but the next 2 lines are the important bit: copyright statement and
license.
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't mix code with copyright and license. Also don't use absolute line numbers. A coding declaration also gets thrown in here in some cases and there can be more than one copyright line. I think we should have copyright and license in separate subsections.

.. code-block:: python

#!/usr/bin/python
# Copyright © 2017 [REPLACE THIS]
Copy link
Contributor

Choose a reason for hiding this comment

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

Use (c) instead of ©

additions to the module are allowed to add an additional copyright line. This
is especially true of rewrites, but original authorship copyright messages
should be preserved. If applicable, contributors may have their rights assigned
to their employer for example: "Copyright © 2017 Red Hat, Inc."
Copy link
Contributor

Choose a reason for hiding this comment

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

Still don't like this paragraph's phrasing. Perhaps more like this:

Every module must include a copyright line with the original copyright holder. Major additions to the module (for instance, rewrites) may add additional copyright lines but, pre-existing copyright lines should be preserved. Code from Ansible is standardising on using the line "Copyright (c) 2017, Ansible Project" which covers all contributiors. Any legal questions need to review the source control history so the copyright header does not need to be exhaustive.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What does "Code from Ansible" mean here? Is that "Ansible, the professional team that gets paid to write Ansible code" or "Code from Ansible contributors"? Maybe "The Red Hat/Ansible team is standardising ....." would be better?

@abadger
Copy link
Contributor

abadger commented Jul 17, 2017 via email

@ryansb
Copy link
Contributor Author

ryansb commented Jul 18, 2017

I see what you mean. I added some more wording recommending that as the standard copyright header.

@ryansb ryansb requested a review from alikins July 18, 2017 13:49
is especially true of rewrites, but original authorship copyright messages
should be preserved. If applicable, contributors may have their rights assigned
to their employer for example: "Copyright © 2017 Red Hat, Inc."
Every file with a copyright line with the original copyright holder. Major
Copy link
Contributor

Choose a reason for hiding this comment

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

Please change "with" to "should have"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@ansibot ansibot added the needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. label Jul 18, 2017
@ryansb ryansb merged commit beca565 into ansible:devel Jul 19, 2017
@ryansb ryansb deleted the copyright-info-dev-guide branch July 19, 2017 10:58
@ansibot ansibot added docs This issue/PR relates to or includes documentation. and removed docs_pull_request labels Mar 4, 2018
@ansible ansible locked and limited conversation to collaborators Apr 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
affects_2.4 This issue/PR affects Ansible v2.4 docs This issue/PR relates to or includes documentation. needs_revision This PR fails CI tests or a maintainer has requested a review/revision of the PR. support:core This issue/PR relates to code supported by the Ansible Engineering Team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants