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

Have RefasterTemplateCollection verify template test class names #233

Merged
merged 3 commits into from
Sep 29, 2022

Conversation

Stephan202
Copy link
Member

@Stephan202 Stephan202 commented Sep 10, 2022

As suggested here.

Suggested commit message:

Have `RefasterTemplateCollection` verify template test class names (#233)

@Stephan202 Stephan202 added this to the 0.2.1 milestone Sep 10, 2022
@Stephan202 Stephan202 requested a review from rickie September 10, 2022 17:58
@Stephan202 Stephan202 force-pushed the sschroevers/validate-template-test-class-names branch from a604b35 to 32a4716 Compare September 18, 2022 11:59
@Stephan202 Stephan202 marked this pull request as ready for review September 18, 2022 11:59
@Stephan202 Stephan202 force-pushed the sschroevers/validate-template-test-class-names branch from 32a4716 to 8323250 Compare September 18, 2022 14:04
@rickie rickie modified the milestones: 0.3.0, 0.4.0 Sep 19, 2022
Copy link
Member

@rickie rickie left a comment

Choose a reason for hiding this comment

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

Added a tiny commit.

Nice improvement, also funny that it flagged an existing issue.

🚀

Was trying to find an alternative suggested commit message mentioning both input and output but did not succeed haha. Current one looks good 👍🏻.

}
}

// This is a comment to appease CheckStyle.
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
// This is a comment to appease CheckStyle.
// This is a comment to appease Checkstyle.

Also below 😉.

For when we get back to this PR.

describeMatch(
typeDeclaration,
SuggestedFix.prefixWith(
typeDeclaration, "/* ERROR: Unexpected declaration. */\n")));
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
typeDeclaration, "/* ERROR: Unexpected declaration. */\n")));
typeDeclaration, "/* ERROR: Unexpected token. */\n")));

Would "token" be more accurate, according to the docs of getTypeDecls:

     * The list may also include empty statements resulting from
     * extraneous semicolons.

Copy link
Member Author

Choose a reason for hiding this comment

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

I like token 👍. Rebased and added a commit.

@Stephan202 Stephan202 force-pushed the sschroevers/validate-template-test-class-names branch from b342377 to 0e0fff7 Compare September 29, 2022 06:28
@Stephan202
Copy link
Member Author

Build failed due to a read timeout connecting to Jitpack. Also affected the most recent build on master, I see.

@Stephan202 Stephan202 force-pushed the sschroevers/validate-template-test-class-names branch from 0e0fff7 to c1e8bb6 Compare September 29, 2022 09:48
@Stephan202
Copy link
Member Author

Rebased and the build now passed. Tnx for the extra review @ferdinand-swoboda! Will merge.

@Stephan202 Stephan202 merged commit 2ba7bf9 into master Sep 29, 2022
@Stephan202 Stephan202 deleted the sschroevers/validate-template-test-class-names branch September 29, 2022 09:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants