Skip to content
This repository has been archived by the owner on Mar 21, 2021. It is now read-only.

Standardize false values for string fields #283

Closed
deepu105 opened this issue Nov 11, 2018 · 16 comments
Closed

Standardize false values for string fields #283

deepu105 opened this issue Nov 11, 2018 · 16 comments

Comments

@deepu105
Copy link
Member

deepu105 commented Nov 11, 2018

Currently we use a mix of false and no to disable things. We should have a standard way of doing this with either false or no everywhere.

For example see serviceDiscoveryType and cacheProvider

application {
  config {
    baseName notification,
    applicationType microservice,
    packageName com.jhipster.demo.notification,
    serviceDiscoveryType false,
    authenticationType jwt,
    databaseType mongodb,
    devDatabaseType mongodb,
    prodDatabaseType mongodb,
    cacheProvider no,
    enableHibernateCache false,
    buildTool gradle,
    serverPort 8083
  }
  entities Notification
}
@deepu105
Copy link
Member Author

I think we should stick with no or none for falsey values instead of false

@MathieuAA
Copy link
Member

MathieuAA commented Nov 11, 2018 via email

@deepu105
Copy link
Member Author

its already broken actually, so i'm doing a fix now to restore old behaviour

@deepu105
Copy link
Member Author

closed the wrong one

@deepu105 deepu105 reopened this Nov 11, 2018
@MathieuAA
Copy link
Member

MathieuAA commented Nov 11, 2018

This concerns everyone @jhipster/developers. I agree with @deepu105's proposal.
false will also be accepted, as long as it concerns a boolean.

If everyone's okay with no, please say so :) If you have any objection, please reconsider tell us why.


Seems none is better!

@DanielFran
Copy link
Member

I'd prefer none (example for cache provider) when it is not a boolean.

But what is important is the consistancy, so if no is the one choosen it is not a problem.

@murdos
Copy link
Contributor

murdos commented Nov 11, 2018

FYI there's a related discussion on jhipster/generator-jhipster#8608
For non boolean parameter none seems more appropriate.
For boolean parameter I've no problem with true/false. But if we choose no, the opposite value should also be changed from true to yes.

@deepu105
Copy link
Member Author

Booleans can remain true/false, I'm only talking about String with no/none/false

@deepu105 deepu105 changed the title Standardize false values Standardize false values for string fileds Nov 11, 2018
@murdos murdos changed the title Standardize false values for string fileds Standardize false values for string fields Nov 11, 2018
@murdos
Copy link
Contributor

murdos commented Mar 9, 2019

@MathieuAA could you do that for JHipster V6? I could handle the changes in generator-jhipster if needed, but I'm not sure how/where things should be changed here.

@MathieuAA
Copy link
Member

@murdos Yep, I'll find time this week to do it.

@murdos
Copy link
Contributor

murdos commented Mar 20, 2019

@MathieuAA : any update on the subject? If you don't have the time, could you tell me where would be the best place in the code to add the standardization of the provided value, as suggested by @deepu105 :
jhipster/generator-jhipster#8608 (comment)

@MathieuAA
Copy link
Member

Yeah I got side-tracked this past few weeks.
You can create an utils file for this, like OptionUtils, that has the method mentioned.

@MathieuAA
Copy link
Member

I think this is a good time to do it, I want to release JCore v6 (including quite a few breaking changes) and this issue is just yelling at me to get closed.
I'm gonna see what needs to be done for this one.

@deepu105
Copy link
Member Author

deepu105 commented Nov 3, 2019 via email

@MathieuAA
Copy link
Member

Agreed.
About the merging: I give up on lerna. It stops parsing a package's commits after a while, giving no clear error message. Maybe someone has to try again, or we could look for other solutions.

@MathieuAA
Copy link
Member

Closing this in favor of jhipster/generator-jhipster#8608.

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

No branches or pull requests

4 participants