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

bugfix - mix of int and long #244

Merged
merged 2 commits into from
Apr 6, 2018
Merged

bugfix - mix of int and long #244

merged 2 commits into from
Apr 6, 2018

Conversation

MFlisar
Copy link
Contributor

@MFlisar MFlisar commented Feb 7, 2018

solves #243

@CLAassistant
Copy link

CLAassistant commented Feb 7, 2018

CLA assistant check
All committers have signed the CLA.

@janheinrichmerker
Copy link
Owner

Seems like Travis Ci is having some license issues, I'll try to get it to work later today.

@MFlisar
Copy link
Contributor Author

MFlisar commented Feb 7, 2018

I've accepted the license while the check was running, don't know if I can retrigger travis?

@janheinrichmerker
Copy link
Owner

@MFlisar it's not because of the CLA but because of some Android support library licenses.
I've had this problem before with Travis CI and I do know how to resolve it. Just need to get home to fix it.

@MFlisar
Copy link
Contributor Author

MFlisar commented Feb 7, 2018

you're right, did not read exactly enough... There's no need to hurry anyway

@meierjan
Copy link
Contributor

meierjan commented Mar 28, 2018

I think once #242 is merged he can pull the changes and travis will succeed again.

@janheinrichmerker
Copy link
Owner

@meierjan I'm going to restart the Travis build. Let's see if it works out then.

@meierjan
Copy link
Contributor

@heinrichreimer this branch needs to get the changes from the master to have a working build. @MFlisar can u do that?

@janheinrichmerker
Copy link
Owner

@MFlisar Could you please merge https://github.com/MFlisar/material-intro/pull/1 to fix issues with the Travis CI build?

@MFlisar
Copy link
Contributor Author

MFlisar commented Apr 6, 2018

@heinrichreimer
Merged my fork with the current master branch... Have not tested it though...

Copy link
Contributor

@meierjan meierjan left a comment

Choose a reason for hiding this comment

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

  • getLong(ARGUMENT_ID) is just consequent as all set-calls do setLong(ARGUMENT_ID)
  • ARGUMENT_ID is private so it can't be referenced outside of this class - can't brake anything
  • getSlideId() seems to be unused inside of this project - so this will brake nothing

Consideration:

  • getSlideId() will brake builds for users (i.e. if they assign it to an int variable)

@janheinrichmerker janheinrichmerker merged commit 7221502 into janheinrichmerker:master Apr 6, 2018
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.

4 participants