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

Exclude generated sources from error-prone #1571

Merged
merged 3 commits into from
Nov 27, 2020
Merged

Conversation

schlosna
Copy link
Contributor

@schlosna schlosna commented Nov 26, 2020

Before this PR

As seen in excavator bump PR palantir/tritium#925 , the changes in #1568 do not seem to handle cases that previously were excluded such as build/metricSchema/generated_src used by metric-schema which is causing the baseline bump excavator on tritium to fail compilation: https://app.circleci.com/pipelines/github/palantir/tritium/906/workflows/7c65b97b-b475-46fc-a1e1-b5a4bf7198a0/jobs/11565

After this PR

==COMMIT_MSG==
Exclude generated sources from error-prone
==COMMIT_MSG==

Closes #1570

Possible downsides?

Probably want some additional test cases.

@changelog-app
Copy link

changelog-app bot commented Nov 26, 2020

Generate changelog in changelog/@unreleased

Type

  • Feature
  • Improvement
  • Fix
  • Break
  • Deprecation
  • Manual task
  • Migration

Description

Exclude generated sources from error-prone

Check the box to generate changelog(s)

  • Generate changelog entry

@policy-bot policy-bot bot requested a review from iamdanfox November 26, 2020 18:10
Copy link
Contributor

@iamdanfox iamdanfox left a comment

Choose a reason for hiding this comment

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

Ah thanks for rescuing this! I wanted to make those regexes really specific, because I figured there was a danger of someone defining a class com.palantir.foundry.build.Whatever and getting it unintentionally excluded - tests definitely worth it

@bulldozer-bot bulldozer-bot bot merged commit 44682aa into develop Nov 27, 2020
@bulldozer-bot bulldozer-bot bot deleted the ds/generated branch November 27, 2020 17:01
static String excludedPathsRegex() {
// don't want backslashes on windows to break our regex
String separator = File.separator.contains("\\") ? Pattern.quote("\\") : File.separator;
return String.format(".*%s(build|generated_.*[sS]rc|src%sgenerated.*)%s.*", separator, separator, separator);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@iamdanfox we might be able to remove build from regex here, or might want to constrain the pattern to avoid falsely matching say com/foo/build/Bar.java.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah this feels a bit dangerous. I'd suggest we do something like build.*generated since almost all code-gen plugins I've seen put source files on a path with a generated component.

@robert3005
Copy link
Contributor

robert3005 commented Nov 30, 2020

Just an FYI, had a project with generated sources in build/generated-src/antlr/. Going to change that to _. Womp, looks I am not following the pattern

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3.57.1 breaks error-prone generated excluded paths
4 participants