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

Some change for normal usable #5

Merged
merged 8 commits into from Nov 28, 2012

Conversation

Projects
None yet
2 participants
@fwolf
Contributor

fwolf commented Nov 25, 2012

This is a good project, but some far for normal usable, at lease for me.
So I'm trying to make it workable for me.

@glasserc

This comment has been minimized.

Owner

glasserc commented Nov 26, 2012

I appreciate your efforts, but I think changes will be necessary before I can merge these patches. I will comment on each.

@glasserc

This comment has been minimized.

Owner

glasserc commented Nov 26, 2012

I don't think there's a need for git commit messages to contain "add", "chg", or "fix" tags.

@glasserc

This comment has been minimized.

Owner

glasserc commented Nov 26, 2012

On balance, if you changed the commit messages, I'd merge the first four of these commits. The last one has some problems -- I'll comment there.

@fwolf

This comment has been minimized.

Contributor

fwolf commented Nov 26, 2012

I've change patch and remove tags :-)

@fwolf

This comment has been minimized.

Contributor

fwolf commented Nov 26, 2012

Bibliographic :date :slug supported, hope you like it.

@fwolf

This comment has been minimized.

Contributor

fwolf commented Nov 26, 2012

As your wish, I've fix fwolf@befa429 in patch fwolf@6704ead .

There are several patch between these 2 commit, maybe hard to rollback for me,
can you merge them all ?

Or, may you suggestion me better way to rewind without lost these middle commit ? thanks.

@glasserc

This comment has been minimized.

Owner

glasserc commented Nov 27, 2012

I would prefer separate pull requests. You can create new branches using git checkout -b. Create a new branch for commits 1-4, a new branch for 6-7, a new branch for 8-9, one for 10, and one for 11. For each branch, you can do git rebase -i to choose only those patches. The other branches won't be changed when you do this. I don't think these commits depend on each other that much (but I haven't read all of them). You can do gitk --all to see where all the branches are. If you open pull requests for each group of patches, I will be able to merge just the ones I like. Plus, you will be able to discard patch 5. I like this because I don't know if I want to merge 8 or 9.

@fwolf

This comment has been minimized.

Contributor

fwolf commented Nov 27, 2012

OK, I've update these commit, here is summary by origin #id:

- 1-4 not touched
- 5 is dropped
- 6-7 is not touched
- 8-9 moved to my dev branch https://github.com/fwolf/rst2wp/tree/dev, pick or discuss later
- 10-11 is not touched

I'm not using git rebase completely,
instead, I export patch, rebase and skip 5+, then import or modify by these patch files.

Do you think create many branch and push to github and then send pull request is complicate ?
I think can work on dev branch, and merge patch want to pull to master branch and send pull request,
and 1 pull request at one time, when accepted, merge next patch(s) and send pull request again.

Is this way ok ? I'll try later.

@glasserc

This comment has been minimized.

Owner

glasserc commented Nov 27, 2012

One other thing is that the first line of git commit messages shouldn't end in a period. (This is very minor.) Make the minor change to "Better fields insert" and I'll merge this.

@fwolf

This comment has been minimized.

Contributor

fwolf commented Nov 28, 2012

Wish all goes fine this time.

glasserc added a commit that referenced this pull request Nov 28, 2012

Merge pull request #5 from fwolf/master
Minor cleanups and robustness

@glasserc glasserc merged commit c6db7f5 into glasserc:master Nov 28, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment