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

#27 test #50

Merged
merged 2 commits into from Sep 11, 2017
Merged

#27 test #50

merged 2 commits into from Sep 11, 2017

Conversation

cleydyr
Copy link

@cleydyr cleydyr commented Jul 21, 2017

No description provided.

@mgaruccio
Copy link
Contributor

I like the implementation, but I still feel (probably biasedly) that #46 solves the original issue better.

The test's are only not passing because #47 has not been merged yet, once the node version is updated in Travis all tests will pass correctly.

@cleydyr
Copy link
Author

cleydyr commented Jul 23, 2017

Does the implementation of #46 work for associating data to existing cards or is it used just when creating new cards? I haven't confirmed it yet, but from the API description it seems that you can just copy or create new cards with that method, not update it.

@mgaruccio
Copy link
Contributor

Ahh good point would only allow for including it on a new card, rather than on something already existing. so this is definitely worthwhile.

@cleydyr
Copy link
Author

cleydyr commented Jul 24, 2017

Yeah. This would allow for updating existing cards that didn't have a due date defined at creation time, but need it after they are created. #46 is just what I needed for my use case, to be honest (I know the due date at creation time). But the functionality of this PR will surely be requested though.

@mgaruccio
Copy link
Contributor

Agreed, although at this point I'm not sure either will be merged, #46 has been waiting for a while.

@norberteder
Copy link
Owner

Hey, Thank you so much for your work! I've merged #46 and #47 today. I hope that helps so far. A new version will be released soon.

@norberteder norberteder merged commit b543d84 into norberteder:master Sep 11, 2017
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

3 participants