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

Default -p option for CDQI tool #57

Closed
patgoley opened this issue May 16, 2017 · 5 comments
Closed

Default -p option for CDQI tool #57

patgoley opened this issue May 16, 2017 · 5 comments

Comments

@patgoley
Copy link
Contributor

If I run the tool without -p, I get compiler errors on my Entity conformance extensions. The errors state typealias CDQIAttribute must be public because it matches a requirement in protocol Entity. Works fine if I run with -p. Compiled using Xcode 8.3.2, Swift 3.1

@invoy-ghigley
Copy link

Yes, I've seen this problem myself. It only occurs when I use the auto-generated "invisible" model classes with Core Data. These classes seem to be public by default. Conversely, if you create a "visible" model class, this is internal by default, which means defaulting -p will give compiler warnings.

If we make -p default, we need to add a new flag to turn it off. I sometimes want to create "visible" model classes because I override methods such as awakeFromInsert.

Thoughts?

@invoy-ghigley
Copy link

invoy-ghigley commented May 16, 2017

I have no problem with public being the default, by the way. I suspect that most people will use the auto-generated "invisible" model classes, so this is a sensible default.

Maybe we need something like this:

cdqi --visibility internal Model
cdqi -v public Model # This is the default

We would leave the -p flag as an alias for --visibility public for backwards compatibility reasons, however, making it the default effectively renders it a no-op.

Does this sound reasonable?

@Revolucent
Copy link
Member

@patgoley By the way, if you like this proposal, would you mind submiting a pull request? :)

@patgoley
Copy link
Contributor Author

@Revolucent Absolutely, just wanted to get your thoughts first. Will submit one soon. Thanks!

@Revolucent
Copy link
Member

@patgoley As I said in #54, I want to create another project for the cdqi tool and make you the maintainer, assuming it's a responsibility you want to take on. Then we'd alter the main CDQI readme with instructions for installing it as a gem.

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

No branches or pull requests

3 participants