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

Rename Plain reporting to Txt. #1184

Closed
wants to merge 1 commit into from
Closed

Rename Plain reporting to Txt. #1184

wants to merge 1 commit into from

Conversation

vanniktech
Copy link
Contributor

Fixes #1111

@vanniktech
Copy link
Contributor Author

I think this needs one of the many open PRs who fix the master build.

assertThat(plainReport.kind).isEqualTo(PlainOutputReport::class.java.simpleName)
assertThat(plainReport.path).isEqualTo(Paths.get("/tmp/path2"))
it("it should properly parse TXT report entry") {
val textRepot = reports[1]
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor: typo!

@@ -25,7 +25,7 @@ internal class ReportsSpec : Spek({
val args = arrayOf(
"--input", "/tmp/must/be/given",
"--report", "xml:/tmp/path1",
"--report", "plain:/tmp/path2",
"--report", "text:/tmp/path2",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should this not be txt? I'd hope the test also fails with this current version? (After a rebase, master should be green again).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Whops. Yes will fix tomorrow. Had this first but my master was super old and I screwed up when resolving the changes.

@arturbosch
Copy link
Member

Looks good! Please add this change to the (not yet created) migrate section of RC10, so I won't forget this.
Also please amend your lat commit message or squash your commits (well I can do it too) to confirm to our contribution guide.

@vanniktech
Copy link
Contributor Author

Looks good! Please add this change to the (not yet created) migrate section of RC10, so I won't forget this.

Can you elaborate?

Also please amend your lat commit message or squash your commits (well I can do it too) to confirm to our contribution guide.

You can do that if you select Squash and merge

@Mauin
Copy link
Collaborator

Mauin commented Oct 3, 2018

@vanniktech The migration guide for the next release is currently in the commented out part at the top of: https://raw.githubusercontent.com/arturbosch/detekt/master/docs/pages/changelog.md.

@vanniktech
Copy link
Contributor Author

Should be good to go now. I believe.

@arturbosch
Copy link
Member

Rebased and merged this manually, thanks!

@arturbosch arturbosch closed this Oct 5, 2018
@arturbosch arturbosch added this to the RC10 milestone Nov 2, 2018
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.

None yet

4 participants