ctubbsii commented on issue #3086: URL: https://github.com/apache/accumulo/issues/3086#issuecomment-1320087679
That's very cool. I didn't know about this. Some points to consider: 1. This makes it easier for unreviewed malicious code to sneak in via an innocuous pull request; we still have to review the contents of these directories before merging if they appear in a PR, and 2. GitHub's UI shows these suppressed files in a format that is very similar to their automatic suppression of large diffs that still need to be reviewed, so you still have to scroll past them in the UI and read the message for each one to determine if they need to be reviewed individually. 3. Since this only affects how generated files are displayed in GitHub, it's still nice to have a separate commit in the git history for when one is reviewing the logs on the command-line or in other git tools. GitHub's UI for this could be a little better. I'd rather have toggle to automatically hide/show all generated files, similar to GitHub's hide/show whitespace changes toggle. Also, I'd really like the generated code to be in a separate commit for the command-line history when we make big changes to the thrift files. My conclusion: I think using this is probably better than not using it, but it doesn't help with everything we care about. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
