Skip to content

Drop Google repo–priority in CONTRIBUTING - #246

Merged
aaronliu0130 merged 4 commits into
developfrom
uncouple
Feb 17, 2024
Merged

aaronliu0130 merged 4 commits into
developfrom
uncouple

Conversation

@aaronliu0130

@aaronliu0130 aaronliu0130 commented Feb 4, 2024 •

Copy link
Copy Markdown
Member

See google/styleguide#528 (comment). Google is no longer maintaining the public version of cpplint, and pretty much everything in their repo's PRs and issues about cpplint have gone unimplemented. Thus, recommending people do stuff there first is sort of useless, and there's not much use confining ourselves to Google's feature set.

I have replaced it with something that's sort of a to-do list + a list of things that probably shouldn't be done.

This may not be needed if the promised publishing of Google's internal version arrives.

Note that I didn't mean to say "Google-favoring" in a derogatory way. Better wording suggestions are welcome.

(When merging, please squash and merge)

@aaronliu0130
aaronliu0130 requested review from jayvdb and tkruse February 4, 2024 02:02
Comment thread CONTRIBUTING.rst Outdated
Comment thread CONTRIBUTING.rst Outdated
Comment thread CONTRIBUTING.rst Outdated
Comment thread CONTRIBUTING.rst Outdated
The difference between this cpplint and Google should remain so small that anyone can at a glance see there is no added change that could be regarded as a security vulnerability.
* Reducing interference with other popular tools
* Update cpplint for modern python versions
* Converting setup.py to a pyproject.toml

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to be careful here - this isnt difficult to do badly, and inviting people to do this will likely result in it being done badly. We want to maintain Python backwards compatibility as much as possible because people using cpplint are likely to be using much older versions of Windows/Linux for their build environment because they have some third-party SDK that isnt available on newer OS versions.

@aaronliu0130 aaronliu0130 Feb 4, 2024 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

AFAIK pyproject.toml is compatible with 3.6.
How about

Suggested change
* Converting setup.py to a pyproject.toml
* Converting setup.py to a pyproject.toml
* Preserve backwards-compatibility without sacrificing readability

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

My point is this document is not the place to ask for someone to do "Converting setup.py to a pyproject.toml".
An issue should be created about this enhancement, and the needs to be done carefully according to that issue, probably by one of the maintainers.

@aaronliu0130 aaronliu0130 Feb 4, 2024 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure how to incorporate "improve compatibility" now. It's not something we can just create an issue about, yet it's also a goal, and making it the only goal would look weird. "Add features and provide a PyPI package" seems like a given for almost any Python project.

Comment thread CONTRIBUTING.rst
@aaronliu0130
aaronliu0130 requested a review from jayvdb February 4, 2024 19:42
Comment thread README.rst
* Overriding repository root auto-detection via --repository
* Support ``#pragma once`` as an alternative to header include guards
* ... and a few more (most of which are open PRs on upstream)
* ... and `quite a bit <https://github.com/cpplint/cpplint/blob/master/changelog.rst>`_ more

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

this should point to the develop branch

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

When we change the branches to the same one, I feel like we should name that branch master. Plus, we'd merge all released changes in develop to master anyway.

@aaronliu0130 aaronliu0130 changed the title Replace Google-favoring with recommended contributions in the CONTRIBUTING guide Drop Google repo–priority in CONTRIBUTING Feb 17, 2024
@aaronliu0130
aaronliu0130 merged commit cf82efe into develop Feb 17, 2024
@aaronliu0130
aaronliu0130 deleted the uncouple branch February 17, 2024 23:45
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.

2 participants