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

pre-commit: Ignore time stamps in po files #708

Open
wants to merge 4 commits into
base: 2.5
Choose a base branch
from

Conversation

daschuer
Copy link
Member

I revert normally manually time stamp only changes, to avoid cluttering a commit wit 4100 changed files. This is tedious and error prone. This PR automates it.

@daschuer daschuer requested a review from Holzhaus December 29, 2024 11:35
daschuer added a commit that referenced this pull request Dec 29, 2024
…p has changed

It helps to create meaningful commits, instead of up to 4100 changed files.
@daschuer
Copy link
Member Author

It has been used here:
70a7984

Copy link
Member

@Swiftb0y Swiftb0y left a comment

Choose a reason for hiding this comment

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

LGTM 🤷 @Holzhaus you're know python better than me, do you have time for a quick look?

)


def revert_po_file(changeset, po_file, logger):
Copy link
Member

Choose a reason for hiding this comment

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

makes no sense to pass a logger here, getLogger is cheap

Comment on lines +61 to +62
proc = subprocess.run(cmd, capture_output=True, text=True)
proc.check_returncode()
Copy link
Member

Choose a reason for hiding this comment

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

looks like you want this:

Suggested change
proc = subprocess.run(cmd, capture_output=True, text=True)
proc.check_returncode()
output = subprocess.check_output(cmd, text=True)

)
parser.add_argument("--from-ref", help="Use changes since this commit.")
parser.add_argument("--to-ref", help="Use changes until this commit.")
parser.add_argument("files", nargs="*", help="Only check these files.")
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
parser.add_argument("files", nargs="*", help="Only check these files.")
parser.add_argument("files", nargs="*", type=pathlib.Path, help="Only check these files.")

return f"{from_ref}...{to_ref}" if to_ref else from_ref


def count_meaningful_changes(changeset, po_file) -> int:
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def count_meaningful_changes(changeset, po_file) -> int:
def count_meaningful_changes(changeset: str, po_file: typing.Iterable[pathlib.Path]) -> int:

proc = subprocess.run(cmd, capture_output=True, text=True)
proc.check_returncode()

with open(po_file, "w") as file:
Copy link
Member

Choose a reason for hiding this comment

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

if po_file is a Path, you can do this:

Suggested change
with open(po_file, "w") as file:
with po_file.open(mode="w") as file:

Comment on lines +82 to +83
# Remove the first entry which is the script file name itself
files_to_process = args.files[1:] if args.files else []
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this is correct. Are you confusing this with sys.argv?

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.

3 participants