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

Fix-up of PR #11402: Redeem the copyright holders' names for source/gui/inputGestures.py after its split from source/gui/settingsDialogs.py #12386

Merged
merged 2 commits into from Jun 21, 2022

Conversation

JulienCochuyt
Copy link
Collaborator

@JulienCochuyt JulienCochuyt commented May 10, 2021

Link to issue number:

Fix-up of PR #11402

Summary of the issue:

In PR #11402, part of source/gui/settingsDialogs.py has been extracted to the new source/gui/inputGestures.py for good reasons.
However, in the process, the name of the copyright holders for this portion of the code has not been reported to the header of the new file.

Description of how this pull request fixes the issue:

Copy all the names from the copryright headers in source/gui/settingsDialogs.py.

Testing strategy:

Known issues with pull request:

Change log entries:

Code Review Checklist:

  • Pull Request description is up to date.
  • Unit tests.
  • System (end to end) tests.
  • Manual tests.
  • User Documentation.
  • Change log entry.
  • Context sensitive help for GUI changes.

…`source/gui/inputGestures.py` after its split from `source/gui/settingsDialogs.py`
@JulienCochuyt JulienCochuyt requested a review from a team as a code owner May 10, 2021 00:25
Copy link
Member

@seanbudd seanbudd left a comment

Choose a reason for hiding this comment

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

Thanks for catching this, I've referenced this with the contributors list on settingsDialog as well and looks good. I've suggested to remove the 2 NV Access developers though.

source/gui/inputGestures.py Outdated Show resolved Hide resolved
@JulienCochuyt
Copy link
Collaborator Author

With admittedly low priority, I'm working on a script to try blame all the revisions of the given range, including those eventually fully superseded.

Copy link
Contributor

@lukaszgo1 lukaszgo1 left a comment

Choose a reason for hiding this comment

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

@seanbudd Since as part of different PR I've verified that the list of contributors is accurate here can we consider applying my suggestion below and merge this? While I agree that having completely precise copyright headers for moved code is not horribly important if we have an complete list it seems a waste not to use it and prefer to use an entire (likely also not completely accurate) header from gui\settingsDialogs.

@@ -1,6 +1,7 @@
# -*- coding: UTF-8 -*-
# A part of NonVisual Desktop Access (NVDA)
# Copyright (C) 2020 NV Access Limited
# Copyright (C) 2013-2020 NV Access Limited, James Teh, Zahari Yurukov, Joseph Lee, Reef Turner,
Copy link
Contributor

Choose a reason for hiding this comment

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

When working on #13294 I've verified that the copyright header herer is accurate, though it seems we don't include names of NV Access employees.

Suggested change
# Copyright (C) 2013-2020 NV Access Limited, James Teh, Zahari Yurukov, Joseph Lee, Reef Turner,
# Copyright (C) 2013-2020 NV Access Limited, Zahari Yurukov, Joseph Lee,

@seanbudd seanbudd marked this pull request as ready for review June 21, 2022 01:40
@AppVeyorBot
Copy link

See test results for failed build of commit 49ea743a24

@AppVeyorBot
Copy link

See test results for failed build of commit 49ea743a24

@AppVeyorBot
Copy link

See test results for failed build of commit ad3fb10987

@seanbudd seanbudd merged commit ae17144 into nvaccess:master Jun 21, 2022
@nvaccessAuto nvaccessAuto added this to the 2022.3 milestone Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants