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

Provide a flag to control generation of the invocation comment #105

Closed
wants to merge 1 commit into from

Conversation

skitt
Copy link

@skitt skitt commented Oct 9, 2023

The invocation comment stores the complete details of all provided arguments, which may include paths to files. These aren't always consistent, which results in unwanted changes in the generated files (especially for CI checking for unexpected changes...).

Since the invocation isn't always useful, provide a flag to control whether the comment is generated. To preserve current behaviour this defaults to true (enabled).

Fixes #104.

The invocation comment stores the complete details of all provided
arguments, which may include paths to files. These aren't always
consistent, which results in unwanted changes in the generated files
(especially for CI checking for unexpected changes...).

Since the invocation isn't always useful, provide a flag to control
whether the comment is generated. To preserve current behaviour this
defaults to true (enabled).

Signed-off-by: Stephen Kitt <[email protected]>
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@ewhauser
Copy link

@skitt - Can you sign the CLA so that this gets merged? Thank you!

Copy link
Contributor

@tchung1118 tchung1118 left a comment

Choose a reason for hiding this comment

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

LGTM. Will be merged once author signs CLA.

@skitt
Copy link
Author

skitt commented Oct 16, 2023

@skitt - Can you sign the CLA so that this gets merged? Thank you!

I’m just waiting for our legal department to check it.

@sodul
Copy link
Contributor

sodul commented Jan 26, 2024

@skitt any progress on the CLA? We are staying at v0.2.0 until there is a new release with this fix.

@skitt
Copy link
Author

skitt commented Jan 29, 2024

@skitt any progress on the CLA? We are staying at v0.2.0 until there is a new release with this fix.

It’s still pending, sorry...

@sodul
Copy link
Contributor

sodul commented Feb 6, 2024

@JacobOaks I've created an alternate PR (#153) against the latest main (no conflicts) and have signed the CLA. This should allow for mockgen to be released with this issue fixed for good.

FYI, I have contributed to the project before the ownership transfer.

@JacobOaks
Copy link
Contributor

Hey @skitt, closing as superceded by #153. Thanks!

@JacobOaks JacobOaks closed this Feb 7, 2024
@skitt
Copy link
Author

skitt commented Feb 7, 2024

That’s fine @JacobOaks, thanks @sodul for proposing an alternate PR!

@skitt skitt deleted the disable-invocation-comment branch June 17, 2024 15:48
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.

Allow generation of the “Generated by this command” comment to be disabled
7 participants