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

Feature/xml attributes #19

Closed
wants to merge 6 commits into from
Closed

Feature/xml attributes #19

wants to merge 6 commits into from

Conversation

markusressel
Copy link

Hey there,

I modified your nice little library to support more properties using xml attributes.
Would be really awesome if you could think about it as I want to use your library in production (with attributions of course ;D)

Thx,
Markus

markusressel and others added 6 commits July 6, 2018 23:24
updated gradle to 4.4
added attributes for:
- stepperMaxCount
- stepperMinCount
- stepperCount
- stepperEnableSideTap
updated gradle to 4.4
added attributes for:
- stepperMaxCount
- stepperMinCount
- stepperCount
- stepperEnableSideTap
update example app
@DanielMartinus
Copy link
Owner

DanielMartinus commented Jul 10, 2018

Thanks! In order to not look like I've gone completely off the radar I want to let you know I've seen your PR and I will soon test and look more thoroughly at it.

I've also seen all code is formatted in a different way, are you using a different code style? I don't think I enabled ktlint + spotless for this project which I should for these kind of occasions.

@markusressel
Copy link
Author

I've also seen all code is formatted in a different way, are you using a different code style?

Yes this was unintentional though :-( I wanted to keep the code style but I guess my "format code key-combo compulsion" was too strong 😄

@DanielMartinus
Copy link
Owner

DanielMartinus commented Jan 8, 2019

Hi @markusressel,

I'm sorry that I've not spend any time reviewing the pull request after last time. I'm going through the pull requests again. Unfortunately I won't be able to merge this pull request because of all the code formatting that's happening. It's not in par with the formatting I have in mind for this library. Same goes for upgrading the internal dependencies. I appreciate that you upgraded them but those have already been taken care of. I also would have preferred to have the pull request address single issues instead of taking on several issues (such fixing code formatting, upgrading sdk and implementing the feature).

Instead, if you're still in for it, I'd like to discuss what you had in mind to implement as extra xml attributes. I see a couple new ones in this pull request:
maxCount
minCount
count
EnableSideTap
onStep

I think they're a great addition to the library. My suggestion is to create a new pull request with these implemented so that it's a clean slate without all the code formatting and other changes that make the review part a little harder.

Hope you understand. I'm doing it this way because when we merge it the credits are yours as contributor and otherwise I can look at it if you're ok with that. I can imagine it's a while ago and you might have no use for it anymore.

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.

None yet

2 participants