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 to "Your First Block" documentation. #348

Closed

Conversation

aleksandrmelnikov
Copy link

@aleksandrmelnikov aleksandrmelnikov commented Dec 15, 2016

I am targeting this branch, because the documentation update has to do with the block-bundle version 3.2.0. I assume this bundle is available only in the 3.x branch.

Changelog

### Added
- Added documentation about setting up your first block for version 3.2.0 of the block-bundle.

@greg0ire
Copy link
Contributor

Oh wow… 37 commits… I think you need to rebase…

@SonataCI
Copy link
Collaborator

Could you please rebase your PR and fix merge conflicts?

@aleksandrmelnikov
Copy link
Author

To be honest, I have not done very many pull requests via GitHub. Can you give me a suggestion for what I should do?
Close this Pull Request and create a new one with some more steps?

@greg0ire
Copy link
Contributor

greg0ire commented Dec 15, 2016

You should do the following :

  1. Fetch all new commits: git fetch --all. ⬇️
  2. Rebase on what you fetched interactively: ✂️
    1. git rebase -i origin/3.x, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin repository.
    2. A window will show up with many lines, remove every line but the last one, which should correspond to your commit.
    3. If you run into a conflict: 💥
      1. fix it with git mergetool. 😷
      2. continue on your merry way with git rebase --continue. ⏩
  3. Force push to overwrite all this : git push --force. ⬆️

Close this Pull Request and create a new one with some more steps?

Try never to do that please ;) . You can always overwrite things thanks to git push --force

@aleksandrmelnikov
Copy link
Author

Thank you for instructions, I will hopefully get this right soon :)

@greg0ire
Copy link
Contributor

greg0ire commented Dec 16, 2016

I see you tried again, but it looks like you forgot the --force option when pushing ;) If you try again, make sure to remove the merge commit. Alternatively, you can simply do this:

  1. Forget everything you did : git reset --hard 3.x
  2. Recover your commit : git cherry-pick c4c7e53
  3. Force push git push --force

It's a bit more simple :)

@aleksandrmelnikov
Copy link
Author

Augh, thank you. I went through like 19 merge conflicts and I was hoping that would have done it. I'll try your new instructions. Thank you for the quick response.

@aleksandrmelnikov
Copy link
Author

Hmm, I feel like I did your instructions on the wrong branch. I did them for 3.x and it was a lot cleaner, but the PR is for patch-1?

@greg0ire
Copy link
Contributor

First step is resetting patch-1 to 3.x, making sure it's the same again. Step 3 is force pushing patch-1. So step 0 would be: make sure you are on patch-1 : git checkout patch-1

@greg0ire
Copy link
Contributor

Also, if you want to better understand what is happening, use git log --graph --decorate --all. It should give you a better view.

@greg0ire
Copy link
Contributor

greg0ire commented Dec 16, 2016

After step 1, you should be on 150d8d5 (or maybe an older version, shouldn't be too serious)

@aleksandrmelnikov
Copy link
Author

Thank you kindly for your patient support. I think I got it pushed up correctly now? I totally did miss step 0.

@greg0ire
Copy link
Contributor

greg0ire commented Dec 16, 2016

Yaaaay! Yes you did! Good job!

@greg0ire
Copy link
Contributor

Please run git commit --amend, and change "Defalut" to "Default", and then git push --force. Should be a piece of cake now ;)

I ran into this while trying to create my first block in the Sonata Admin bundle.
The documentation at this point did not work for version 3.2.0 of the Block Bundle, so I added a note about how to make it work.
@aleksandrmelnikov
Copy link
Author

Of all the things, I even had a typo, hahahaha. But yes, a lot easier now :). Again, I very much appreciate the help.

@greg0ire
Copy link
Contributor

And I'm glad you didn't give up! @sonata-project/contributors , please review!

@aleksandrmelnikov
Copy link
Author

I refused to let my attempt at improving documentation languish ;)

@greg0ire
Copy link
Contributor

Way to go!

@OskarStark
Copy link
Member

I would rather change the setDefaultSettings() to configureOptions() instead of adding a note
what do you think @greg0ire ?

@greg0ire
Copy link
Contributor

greg0ire commented Jan 5, 2017

You're right @OskarStark ! @aleksandrmelnikov , please do that, and optionally invert the note :

for block bundle < 3.2, please use …

Copy link
Contributor

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

please invert the note and change the doc

@greg0ire
Copy link
Contributor

ping @aleksandrmelnikov

@OskarStark
Copy link
Member

I created a new PR in #373

@OskarStark OskarStark closed this Mar 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants