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.

Sign up for GitHub

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

Jump to bottom

Introduce DefaultValue type #3928

Merged
merged 7 commits into from Jan 23, 2022
Merged

Introduce DefaultValue type #3928

merged 7 commits into from Jan 23, 2022

Conversation

marschwar
Copy link
Contributor

@marschwar marschwar commented Jul 1, 2021 •
edited

This PR resolves #3910. The change set is greater than I anticipated but I think the collection and the generation are better separated now and there is less string manipulation magic involved.

@codecov
Copy link

codecov bot commented Jul 1, 2021 •
edited

Codecov Report

Merging #3928 (6609e69) into main (9471385) will increase coverage by 84.10%.
The diff coverage is 87.01%.

❗ Current head 6609e69 differs from pull request most recent head d9a6b3b. Consider uploading reports for the commit d9a6b3b to get more accurate results
Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3928       +/-   ##
===========================================
+ Coverage        0   84.10%   +84.10%     
- Complexity      0     3290     +3290     
===========================================
  Files           0      475      +475     
  Lines           0    10808    +10808     
  Branches        0     2001     +2001     
===========================================
+ Hits            0     9090     +9090     
- Misses          0      690      +690     
- Partials        0     1028     +1028
Impacted Files Coverage Δ
...ekt/generator/collection/ConfigurationCollector.kt 72.22% <78.12%> (ø)
...t/generator/collection/RuleSetProviderCollector.kt 76.82% <87.50%> (ø)
...bosch/detekt/generator/collection/DefaultValues.kt 90.00% <90.00%> (ø)
...rbosch/detekt/generator/collection/DefaultValue.kt 95.23% <95.23%> (ø)
...bosch/detekt/generator/collection/Configuration.kt 100.00% <100.00%> (ø)
...sch/detekt/generator/printer/RuleSetPagePrinter.kt 90.38% <100.00%> (ø)
...t/generator/printer/defaultconfig/ConfigPrinter.kt 93.18% <100.00%> (ø)
...lab/arturbosch/detekt/rules/style/ForbiddenVoid.kt 78.57% <0.00%> (ø)
...rturbosch/detekt/rules/naming/FunctionMinLength.kt 94.11% <0.00%> (ø)
...rmatting/wrappers/NoEmptyFirstLineInMethodBlock.kt 100.00% <0.00%> (ø)
... and 470 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9471385...d9a6b3b. Read the comment docs.

@marschwar marschwar force-pushed the feature/type-safe-default-value branch from 8eabb07 to da46d4a Compare July 3, 2021 17:47
@marschwar marschwar changed the title [RFC] Introduce DefaultValue type Introduce DefaultValue type Jul 5, 2021
@marschwar marschwar force-pushed the feature/type-safe-default-value branch from da46d4a to a0a1c2a Compare July 5, 2021 17:02
@marschwar marschwar marked this pull request as ready for review July 5, 2021 17:29
@marschwar marschwar force-pushed the feature/type-safe-default-value branch from a0a1c2a to 1c88c87 Compare July 7, 2021 20:04
@marschwar marschwar marked this pull request as draft July 20, 2021 22:01
@marschwar
Copy link
Contributor Author

This will need to be reworked after #3964 is merged.

@marschwar marschwar force-pushed the feature/type-safe-default-value branch from 1c88c87 to 8184dc7 Compare July 27, 2021 12:51
@github-actions
Copy link

github-actions bot commented Nov 2, 2021

This PR is stale because it has been open 90 days with no activity. Please comment or this will be closed in 7 days.

@github-actions github-actions bot added the stale label Nov 2, 2021
@cortinico
Copy link
Member

This will need to be reworked after #3964 is merged.

Can we follow up on this?

@cortinico cortinico removed the stale label Nov 3, 2021
@marschwar
Copy link
Contributor Author

Sorry, I have been "offline" for a while. Will try to update soon.

@marschwar marschwar force-pushed the feature/type-safe-default-value branch from 8184dc7 to 8cc6367 Compare November 11, 2021 20:16
@marschwar marschwar marked this pull request as ready for review November 11, 2021 21:07
@marschwar
Copy link
Contributor Author

Any thoughts on this? Does this make sense to you?

BraisGabin
BraisGabin approved these changes Jan 20, 2022
View reviewed changes
Copy link
Member

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

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

Sorry for the delay in the review. I didn't saw that this was ready to review. LGTM

chao2zhang
chao2zhang approved these changes Jan 22, 2022
View reviewed changes
...t-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/DefaultValue.kt Outdated
override fun getAsPlainString(): String = defaultValue.toString()
}

private data class ListDefault(private val defaultValue: List<String>) : DefaultValue {
Copy link
Member

Choose a reason for hiding this comment

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

I would call this ListString instead of List

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean ListDefault ➡️ ListStringDefault? If so, I like the idea, but I would prefer StringListDefault.

...r/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationCollector.kt Outdated Show resolved Hide resolved
...t-generator/src/main/kotlin/io/gitlab/arturbosch/detekt/generator/collection/DefaultValue.kt Outdated Show resolved Hide resolved
...erator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleCollectorSpec.kt Show resolved Hide resolved
Markus Schwarz added 4 commits January 22, 2022 11:05
Merge remote-tracking branch 'upstream/main' into feature/type-safe-d…
e53d412 
…efault-value

# Conflicts:
#	detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/collection/ConfigurationSpec.kt
#	detekt-generator/src/test/kotlin/io/gitlab/arturbosch/detekt/generator/collection/RuleSetProviderCollectorSpec.kt
use DefaultValue.of() explicitly
f56fa4e
default getAsPlainString to toString
37a6394
rename ListDefault to StringListDefault
d9a6b3b
@chao2zhang chao2zhang merged commit 8a69676 into detekt:main Jan 23, 2022
@chao2zhang chao2zhang added this to the 1.20.0 milestone Jan 23, 2022
@marschwar marschwar deleted the feature/type-safe-default-value branch January 24, 2022 07:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@BraisGabin BraisGabin BraisGabin approved these changes

@chao2zhang chao2zhang chao2zhang approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.

Refactor Configuration off String type for default value.
4 participants
@marschwar @cortinico @BraisGabin @chao2zhang