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

Easy Config-related cleanups #5388

Closed
2 of 4 tasks
dmlloyd opened this issue Nov 11, 2019 · 13 comments · Fixed by #10440
Closed
2 of 4 tasks

Easy Config-related cleanups #5388

dmlloyd opened this issue Nov 11, 2019 · 13 comments · Fixed by #10440
Labels
good first issue Good for newcomers kind/enhancement New feature or request triage/out-of-date This issue/PR is no longer valid or relevant

Comments

@dmlloyd
Copy link
Member

dmlloyd commented Nov 11, 2019

Description
After #5387, there are several configuration constructs that can be cleaned up in core and extensions.

  • Any @ConfigItem which has a primitive type like int or boolean that has an explicit default value of 0 or false can have the default value removed.
  • Any @ConfigItem with a primitive wrapper type like Integer or Boolean with an explicit default value of 0 or false should be changed to use the primitive type instead and remove the default value.
  • Any @ConfigItem of type Optional<Integer> can be changed to OptionalInt.
  • Any @ConfigItem which is a List type that is always converted to an array using toArray can be changed to just be an array type instead.
@dmlloyd dmlloyd added kind/enhancement New feature or request good first issue Good for newcomers labels Nov 11, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Nov 24, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Nov 26, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Nov 26, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Nov 28, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Nov 28, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Nov 28, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Nov 30, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Dec 1, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Dec 8, 2019
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Jan 18, 2020
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Jan 19, 2020
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Jan 19, 2020
Simulant87 added a commit to Simulant87/quarkus that referenced this issue Jan 22, 2020
@ShubhamRwt
Copy link
Contributor

@dmlloyd Is this issue fixed now? I was planning to work on it.

@dmlloyd
Copy link
Member Author

dmlloyd commented Mar 12, 2020

@ShubhamRwt - the issue is mostly done but the PR (#5489) is out of date and needs a rebase. The original contributor gave up in frustration (this was really our fault) but if you want to start with that commit, rebase it and bring it up to date (and add yourself as Co-authored-by) and submit a new PR, go ahead and do so. Thanks!

@ShubhamRwt
Copy link
Contributor

Ok @dmlloyd. I will rebase it and send a new PR.

@machi1990
Copy link
Member

Thanks @ShubhamRwt

@ShubhamRwt
Copy link
Contributor

Hello @dmlloyd @machi1990 . The previous guy has made some unwanted changes due to which the build is failing. Should I create a new Pull request and make the only required changes?

@dmlloyd
Copy link
Member Author

dmlloyd commented Apr 8, 2020

Go ahead. I'd recommend fixing just one problem per PR at a time; that will make it more likely to get merged quickly and hopefully avoid the troubles that the previous contributor encountered.

@ShubhamRwt
Copy link
Contributor

Yes Sure. That would be great

machi1990 added a commit that referenced this issue Apr 24, 2020
Easy Config-related cleanups #5388
@ShubhamRwt
Copy link
Contributor

@machi1990 I went through the code base but seems like the second part is already resolved. Actually I was stuck with job work lately thats why this didn't got time for the rest over part but I will get it done and sorry for the delay.

@ShubhamRwt
Copy link
Contributor

I will be proceeding with the 3rd pointer mentioned above tomorrow.

@machi1990
Copy link
Member

@ShubhamRwt thanks for the update. No rush, and take your time.

@toniperezfer
Copy link

Hi. I would like to contribute to the project. It seems like there are two unresolved problems yet, right?

@adrianfiedler
Copy link
Contributor

Hi @dmlloyd, @machi1990,
for third task I prepared a PR, could you please check?
#10440

@machi1990
Copy link
Member

Hi @dmlloyd, @machi1990,
for third task I prepared a PR, could you please check?
#10440

Thanks for the PR @adrianfiedler I'll have a look later today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers kind/enhancement New feature or request triage/out-of-date This issue/PR is no longer valid or relevant
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants