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

Clean Code (refractor move --> interfaces) #271

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from
Open

Clean Code (refractor move --> interfaces) #271

wants to merge 6 commits into from

Conversation

thelittlefireman
Copy link

Cleaning code

By @thelittlefirman

@entropitor
Copy link
Contributor

I'm not sure if I like this change because it will break all clients using our library (as they need to implement "other interfaces", they'll need to change their import statements)

@alamkanak what do you think?

@marunjar
Copy link
Contributor

marunjar commented Feb 2, 2016

@thelittlefireman also this PR isn't complete, i'm missing an interfaces folder

@thelittlefireman
Copy link
Author

sorry, it's fix now ;)

@thelittlefireman
Copy link
Author

Conflicts Fixes

 into develop

Conflicts:
	library/src/main/java/com/alamkanak/weekview/WeekView.java
@thelittlefireman
Copy link
Author

again

@entropitor
Copy link
Contributor

@alamkanak Is this something we want to do? We'll break all current clients with this code. (We have 1534 stars on github so that might be a lot of clients that we're breaking).

@thelittlefireman
Copy link
Author

It's just the java convention code ... we cannot put all the code in only one Class, it's a mess ...
And it's just a the imports to change (simple Ctrl+Maj+O to do).
I can readd old interfaces in WeekView Class with deprecated tag if you prefer?

@entropitor
Copy link
Contributor

We could have interfaces in the WeekView class that inherit from the real interfaces. + @deprecated tag. That way I can live with this change.

Can you also update the documentation then?

add deprecated tag and documentations
@thelittlefireman
Copy link
Author

Done :) is that okay right now ?

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.

3 participants