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

PR: Update gitignore, gitattributes, license, authors and security policy #278

Merged
merged 3 commits into from
Dec 3, 2021

Conversation

CAM-Gerlach
Copy link
Member

@CAM-Gerlach CAM-Gerlach commented Nov 8, 2021

Adds/updates various meta-files and syncs with with other Spyder projects; final bit of general maintenance for v2.0.0.

Namely:

  • Update gitignore to modern standardized version used on other Spyder repos, with QtPy-unique additions merged in
  • Add gitattributes from the current standard version from the other Spyder projects, with a few minor tweaks
  • Update license with correct copyright line, in line with that on main Spyder repo and elsewhere, and fix EOL
  • Revise AUTHORS to more closely follow the standard format used on other Spyder repos, and include names and links to the main projects that become QtPy and their authors (from the README), as well as the current maintainer
  • Add a baseline, standardized security policy (which GitHub strongly recommends and will include in their UI), since QtPy is a very important part of the dependency chain for tens of thousands of projects and one of the top ≈200 packages on PyPI

Copy link
Member

@dalthviz dalthviz left a comment

Choose a reason for hiding this comment

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

Thanks @CAM-Gerlach ! I left some comments/questions (mostly on the .gitattributes and .gitignore file). Otherwise LGTM 👍

.gitignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
.gitattributes Show resolved Hide resolved
.gitattributes Show resolved Hide resolved
.gitattributes Show resolved Hide resolved
.gitattributes Show resolved Hide resolved
.gitignore Show resolved Hide resolved
@CAM-Gerlach
Copy link
Member Author

Just to clarify again, the .gitignore and .gitattributes files are sourced directly from the most updated versions of the standardized .gitignore and .gitattributes files we've been using across many/most Spyder projects (including the docs, website, theme, spyder-kernels, gettext-helpers, etc) for at least three years now (as well as many other projects). They themselves are based off standard, widely-used templates, with some further additions based on the needs of various specific Spyder-related projects, and occasionally synced with the latest upstream improvements. I've also made sure to integrate the few instances of .gitignore patterns included in this project's .gitignore that weren't already included in the existing ones.

@dalthviz To address all the "is this necessary" questions, the primarily goal of this PR was to standardize on comprehensive, proven .gitignore and .gitattributes files that correctly cover a wide variety of cases and that have worked well across all our other projects for several years, while reducing the burden of a individually maintaining a patchwork of bespoke, manually-updated files unique to each project. Pragmatically, I believe spending a substantial amount of time bikeshedding over if each section and item is likely to be strictly necessary is likely to cost more time and effort than it could possibly save us, and more importantly negates the purpose of standardizing on a single common version of these files (that can be updated with a simple copy/paste, GitHub Action, or other automated tool in the future) to begin with.

@dalthviz
Copy link
Member

dalthviz commented Nov 9, 2021

Thanks for the explanation @CAM-Gerlach ! Thinking about is still quite strange to me to have stuff related to things that probably will not be used here (maybe is just for me though). What do you think @ccordoba12 ?

@CAM-Gerlach
Copy link
Member Author

CAM-Gerlach commented Nov 9, 2021

Thinking about is still quite strange to me to have stuff related to things that probably will not be used here (maybe is just for me though).

Perhaps, but so long as they're suitably comprehensive, standardized and well-proven, they shouldn't need to ever be looked at again (aside the very occasional update, that with standardized files, can be simple drag and drop). After all, it hasn't been a problem for you before on any of the other Spyder repos using these same standardized files for the past three years, has it? ^_^

Something I've discovered time and again along my own journey as a scientist and a programming is that just going with one standard, consistent approach that works in all probable cases is usually a lot more efficient then spending the time and mental energy thinking about which method I can get away with in each particular case, and the risk of making a mistake if I'm wrong. In this case, this is doubly true since there is no objective cost to just use the standardized, proven version and modest objective benefits, and no clear benefit to spending the considerable time, energy and risk now and in the future determining line by line and section by section whether each may be relevant.

@dalthviz
Copy link
Member

Any feedback on this @ccordoba12 ?

@CAM-Gerlach
Copy link
Member Author

Just to TL;DR my take @ccordoba12 , these are the standard versions we have in many/most of our other modern repos (with a few improvements making sure the .gitignore was a superset of the existing custom QtPy one), which improve diffs, EOL normalization, ignoring editor files, and text/binary handling. So my concerns are ironically for the same reasons as yours on the the _version file change (though in that case, I initially tried to preserve it as much as possible and only relented when it was clear it conflicted with the goal of the PR, whereas here porting the standardized version is the goal of the PR).

@ccordoba12
Copy link
Member

Any feedback on this @ccordoba12 ?

Although I find the changes in .gitattributes and .gitignore a bit of an overkill for a such a simple project as this one, I'm fine with them.

Copy link
Member

@ccordoba12 ccordoba12 left a comment

Choose a reason for hiding this comment

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

Thanks @CAM-Gerlach!

@dalthviz dalthviz merged commit 8285eca into spyder-ide:master Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants