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

feat: adds a feature to provide a list of already added tags #7

Merged
merged 7 commits into from Dec 5, 2020

Conversation

iamdipanshusingh
Copy link
Contributor

Merging this PR will close #6 and will add a feature where the user can set an initial list of tags, which might have been added previously.

This is personally tested by me.
And apart from implementing this feature, I've also updated the README.md accordingly and have modified the code a little bit (have made the if statements shorter). Also, I didn't notice while committing that Android studio has formatted the code, LMK if you want that like before.
LMK, if any further change is required from my end. 🙂

@iamdipanshusingh iamdipanshusingh changed the title feat: adds a feature to provide a list of already added tags [WIP] feat: adds a feature to provide a list of already added tags Nov 19, 2020
@iamdipanshusingh iamdipanshusingh changed the title [WIP] feat: adds a feature to provide a list of already added tags [wip] feat: adds a feature to provide a list of already added tags Nov 19, 2020
@iamdipanshusingh
Copy link
Contributor Author

@eyoeldefare please draft this PR for now... it has some bugs need to fix that

@iamdipanshusingh
Copy link
Contributor Author

iamdipanshusingh commented Nov 19, 2020

@eyoeldefare I've fixed the encountered issue, and have done some more changes:
now, you don't need to define onTag and onDelete if you're passing the tags, the tags will be maintained internally. So, onTag & onDelete are optional now.

Apart from this, your code was breaking if onDelete and onTag are not provided, I've managed that as well.
Lastly, have a look at the assertion I've added.

You can merge it, if you want to. LMK if you want to add something, I'll be happy to hear.

@iamdipanshusingh iamdipanshusingh changed the title [wip] feat: adds a feature to provide a list of already added tags feat: adds a feature to provide a list of already added tags Nov 19, 2020
@eyoeldefare
Copy link
Owner

Hey thanks for the contribution, I will review this soon.

@ondbyte
Copy link

ondbyte commented Nov 25, 2020

would be great if you push it to pub!

1 similar comment
@ondbyte
Copy link

ondbyte commented Nov 25, 2020

would be great if you push it to pub!

@eyoeldefare
Copy link
Owner

would be great if you push it to pub!

I am a bit busy right now due to exam season. I can't promise you to push it to pub but will review it during spring break.

@iamdipanshusingh
Copy link
Contributor Author

@ondbyte till then, you can use my version... I've merged my changes to my forked repo's master branch

Copy link
Owner

@eyoeldefare eyoeldefare left a comment

Choose a reason for hiding this comment

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

Hey thanks for the pr again. I was very busy until just today.
I think everything checks out here.
I will push this to master.

@eyoeldefare eyoeldefare merged commit c3b1682 into eyoeldefare:master Dec 5, 2020
@iamdipanshusingh iamdipanshusingh deleted the dipanshu/feature/tag_list branch December 5, 2020 14:52
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.

feature request: add an option to provide a list of already added tags
3 participants