-
Notifications
You must be signed in to change notification settings - Fork 371
README improvements #131
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
README improvements #131
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clean up and clarification, much appreciated. A lot of the team have been out for the 4th of July week so sorry for the slow response.
Any chance you could squash this series of patches into a single patch?
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Simply adding the plugin to your project isn't recommend as the Version Handler component will activate / enable all subcomponents of the plugin, preventing resolution of conflicting versions of this plugin. This still needs a reference to the Plugin Redistribution section to ensure developers import the plugin correctly when redistributing the plugin. I see Plugin Redistribution has been moved to bullet 3, would it make sense to add a note that step 3 is really important?
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to above, it would be great to make sure users don't miss the redistribution and also should probably avoid having specific version numbers in here.
ad61947 to
e18a844
Compare
Thanks for the feedback. Since the Plugin Redistribution part is important, I've moved it closer to the start of the doc. I think it makes sense to be first, since importing the plugin is the first thing new developers will want to do. It's important they do it correctly, no?
README.md
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to ditch the version number here as it's going to get out of date.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops. Fixed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally looks ok, thanks again. Only one minor fix required as far as I can tell.
e18a844 to
0fdd1a6
Compare
Thanks again.
Uh oh!
There was an error while loading. Please reload this page.
Fixes #129.
I'd like to improve the Plugin Redistribution Background section, but I'm not sure what it should say exactly. Currently it explains the technical details of why the
gvh_disableoption is required, but I don't think it's a very satisfactory explanation.gvh_disablebeing optional at import time, but the Plugin Redistribution section makes it sound mandatory.If the
gvh_disableoption is super-duper important in all situations (for both importing and exporting), then I will mention it in the various Usage sections.