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

Source code: consistency edits for coding style and guidelines #9589

Closed
josephsl opened this issue May 18, 2019 · 4 comments
Closed

Source code: consistency edits for coding style and guidelines #9589

josephsl opened this issue May 18, 2019 · 4 comments

Comments

@josephsl
Copy link
Collaborator

Hi,

Building from #9548:

Is your feature request related to a problem? Please describe.

Since its inception in 2006, NVDA's source code has grown over the years with help from NV Access people and hundreds of contributors. Due to the nature of this project, different people had different coding styles, which eventually led to different spellings, function names, copyright header differences (or no header at all for some files) and other inconsistencies.

Describe the solution you'd like

Because Project Threshold plans to drop backwards incompatible changes that will affect many modules (#7105, for instance), coupled with an ongoing work on removing deprecated code (see #9548), it is proposed to take a look at NVDA source code and edit it for consistency. Candidates include winUser.FindWindow -> winUser.findWindow, adding copyright headers (probably a separate issue) and others.

Describe alternatives you've considered

One alternative is status quo - keep the source code as is. But if this is done, NVDA would present a picture that could encourage inconsistencies in the future.

Additional context

See:

Thanks.

@Brian1Gaff
Copy link

Brian1Gaff commented May 19, 2019 via email

@LeonarddeR
Copy link
Collaborator

I agree with this proposal. However, with regard to winUser, we'd have to discuss whether we follow standard NVDA guidelines (lowerCamelCase) or mimic windows API UpperCamelCase. I guess the inconsistency in winUser is caused by unclarity about the followed approach.

@feerrenrut
Copy link
Contributor

I can certainly understand the desire for more consistent code, however a broad set of changes just for code style is not something we would consider. We are willing to consider breaking compatibility and introducing risk when we must (python3), or when it allows for sufficient new features and opportunities for innovation (speech refactor). I don't think we are willing to introduce risk to rectify code style. Another alternative to prevent further inconsistencies is to introduce a linter to stop new code from introducing style variations. Flake8 can be run on a a diff, we can hook this up to diff against master and fail a PR if the linting does not work. We can provide a build option to run this locally too (thus reducing the feedback loop).

One of the first steps is to decide on the set of rules / configuration for a linter. I think the closer to the standard PEP8 rules the better, being consistent with the rest of the python community has advantages, one of which is (hopefully) better consistency with libraries.

If we introduce something like this, the code base will gradually migrate to that style, but only when people are actively working in an area (and thus understanding it, and thinking about it carefully)

@feerrenrut
Copy link
Contributor

Linking to #6090 since the reasoning for closing is similar.

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

No branches or pull requests

4 participants