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

Update README.md #59

Merged
merged 1 commit into from Apr 12, 2017

Conversation

@supremebeing7
Copy link
Contributor

commented Jan 10, 2017

@zorab47

This comment has been minimized.

Copy link
Owner

commented Feb 4, 2017

I've been using it with older versions of ActiveAdmin (0.6.x and some along the master branch). What was the bug and when might it have been introduced? I don't want to mislead new users of the gem on which version is required.

@supremebeing7

This comment has been minimized.

Copy link
Contributor Author

commented Feb 6, 2017

Hm, I'm not sure when it was introduced. I've been on 1.0.0.pre since coming to this project. I understand your concern. I can try to research a bit and try to nail down when the bug was introduced. If we can't figure that out, would it be worthwhile to change the wording in this PR? Something like "If you experience issues with drag and drop, you may need to specify the version..."

@supremebeing7

This comment has been minimized.

Copy link
Contributor Author

commented Feb 6, 2017

I found this: activeadmin/activeadmin#3776 (comment)
Which makes it sound like this error is sporadic (which is the best kind of error 🎉 ).

I also found this: activeadmin/activeadmin#4198
At the end, one of AA's maintainers says that the fix was backported to v0.6.6.

Either of those could be why you haven't experienced this error. Either way, I think just changing the language in my edits like I suggested above might make the most sense. Let me know what you think, or if you don't think README edits are necessary.

@zorab47

This comment has been minimized.

Copy link
Owner

commented Apr 10, 2017

Agreed, I think a wording update would be wise. Providing this warning could help others avoid similar issues.

@supremebeing7

This comment has been minimized.

Copy link
Contributor Author

commented Apr 11, 2017

Updated wording. Let me know if anything else is needed.

Thanks for the great gem, hopefully this saves someone else some head-scratching.

@zorab47 zorab47 merged commit b017cd3 into zorab47:master Apr 12, 2017
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@zorab47

This comment has been minimized.

Copy link
Owner

commented Apr 12, 2017

Thanks!

@supremebeing7 supremebeing7 deleted the supremebeing7:patch-1 branch Apr 12, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.