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

Fix upgrade from factory girl instructions #1102

Conversation

benkoshy
Copy link

@benkoshy benkoshy commented Jan 31, 2018

What is the main proposed change?

Summary: the proposed change allows folks to easily edit instructions.

Detailed explanation: The previous commit referred the upgrade instructions to a fixed tagged commit via a link. But then how can folks upgrade or improve upon those instructions? Instructions are living/breathing documents subject to change and editing. Hence these instructions must be included in the current HEAD / commit to allow for easy emendation. That's what this commit seeks to do.

The second change

I have also edited the instructions re: upgrading. But, given the above change, you can easily emend those instructions if I've made an error :)

This reverts commit 2fb828b.

A link to a document referred to in a tagged commit does not allow for changes and upgrades made to that particular document. Instructions are a living/breathing document that are subject to changes and improvements. I had originally wanted to improve on the upgrade instructions but could not do so readily, because they were linked to a particular tagged commit. Hence had to revert the change to allow for upgrades to be made. I have incorporated the entire changes in the linked document herein. If a link is to be made, then it must be to a document in the current head, not a fixed commit, so that changes can be readily made.
@composerinteralia
Copy link
Collaborator

composerinteralia commented Jun 12, 2018

Thanks for your contribution!

I don't think the upgrade guide belongs at the top of README.md. It makes it more difficult for first-time users to find the documentation and install details. I would prefer a PR against the 4-9-0-stable branch. The commits related to UPGRADE_FROM_FACTORY_GIRL.md are already on that branch, and it should be just as easy to edit there as it would have been to edit it on master (easier in this case since we will be able to see the diff).

@composerinteralia
Copy link
Collaborator

I'm going to close this for now. Feel free to open a PR against 4-9-0-stable. Many thanks!

@benkoshy
Copy link
Author

I can add to the other branch.

@thoughtbot thoughtbot locked as resolved and limited conversation to collaborators Aug 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants