Skip to content

Conversation

@bileschi
Copy link
Collaborator

@bileschi bileschi commented May 2, 2022

These lint clauses make sense in the original repo these files are vended from. They are not useful here as the linked file does not exist in our repo, and we should not be manually changing these files anyway.

The change is innocuous in open source but the lint clauses trigger warning within Google's internal CI.

A subsequent change to tensorboard/compat/proto/update.sh will disable these lints automatically to prevent their reintroduction.

@bileschi bileschi requested a review from JamesHollyer May 2, 2022 22:14
Copy link
Contributor

Choose a reason for hiding this comment

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

Kind of a nit, but for cases like this, this leaves behind a somewhat confusing fragment with the latter half of the LINT directive. Would be a bit nicer if we either were to strip the entire thing, or leave some explanatory replacement like // Disabled ThenChange block: (

@bileschi bileschi force-pushed the if-this-than-no-thanks branch from dcf7fa6 to e1ae923 Compare May 2, 2022 22:56
@bileschi bileschi changed the title Remove IfChange / ThenChange Lint clause from vended protos Disable IfChange / ThenChange Lint clause from vended protos May 2, 2022
@JamesHollyer
Copy link
Contributor

I wrote the script to make these updates automatically. I think it makes sense to just merge these changes in the same PR.

#5693

@bileschi bileschi closed this May 12, 2022
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