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

Update .gitignore using gitignore.io #681

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

altendky
Copy link
Contributor

No description provided.

@altendky altendky marked this pull request as draft August 27, 2024 15:39
Copy link

Pull Request Test Coverage Report for Build 10580947399

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.891%

Totals Coverage Status
Change from base Build 10575607681: 0.0%
Covered Lines: 12301
Relevant Lines: 14663

💛 - Coveralls


# Remove Cargo.lock from gitignore if creating an executable, leave it for libraries
# More information here https://doc.rust-lang.org/cargo/guide/cargo-toml-vs-cargo-lock.html
Cargo.lock
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe the python wheel would be considered an "executable" in this context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see two topics here. They may be in conflict.

  1. Should we include Cargo.lock file(s) in the repo?
  2. Should we edit the standard output of the gitignore template?

For 1), there is presently a single Cargo.lock in the repo and it is revisioned. For that file, this entry in the configuration is irrelevant unless we remove the file from git. In that case git would not remind us to add it back. This doesn't seem like a critical backstop.

For 2), I guess we can edit it. When someone does an update to include either updated changes from the templates for the groups of ignores selected here, or to add another group such as for another editor, then they will overwrite the manual modification. Odds are the rust template won't change drastically so the manual modification will show in the diff and be noticed. If we were to make the ignore files managed like other general repo files then this exception would probably get more annoying. Especially since the comment suggests it is a conditional thing that would vary and require both additional configuration per rust repo and code to manage the modification.

Given the near zero upside of modifying this and the everpresent issues around modifying generated content, I'm obviously not in favor of it. But, if someone clicks commit below, then so it goes.

Suggested change
Cargo.lock
# Cargo.lock

*.so
*.pyd
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the file extension for native python modules on windows, right? Do we not need this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is covered by *.py[cod].

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