Appending Thunderlink to file

mikehardy / thunderlink

Link to your Thunderbird emails!

Mozilla Public License 2.0

42 stars 14 forks source link

Appending Thunderlink to file #30

Closed MarioKusek closed 5 years ago

MarioKusek commented 5 years ago

I have created implementation for Issue #28. When you have time please look at it.

I have used eslint ;)

Thanks, Mario

mikehardy commented 5 years ago

Oh - worth mentioning that the git history is really messy :-)

What I did in order to test it was actually make a new branch locally, then cherry-pick your last 7 commits to make it clean.

I vastly prefer that over merging - assuming the place you want to merge to is a git remote called upstream my usual workflow is:

have a feature idea
git co master && git fetch upstream && git rebase upstream/master && git push
git co -b new-feature-name
do the work
git push (and usually git will remind you that you need to do a set-upstream or whatever - I just copy-paste that the first time)

It looks like this PR is carrying around 21 commits, with 2 of them being merges, instead of just the 7 commits that actually implement the new changes

It'd be nice if you could clean up the history so this was an easier "rebase and merge" when it's fully working

MarioKusek commented 5 years ago

I have updated code to conform to your comments. Please look at it again when you have time.

mikehardy commented 5 years ago

I was too slow with the comment on the equals sign but I want to go the code change direction instead of the preferences change direction, then this is probably good

MarioKusek commented 5 years ago

I have created commit for backward compatibility with existing templates.

MarioKusek commented 5 years ago

No problem.

Do you want me to clean log history now and create new pull request? Or how do you want me to proceed?

mikehardy commented 5 years ago

I learn new things about git every day, I just figured out how to easily cherry-pick a range (git cherry-pick commitA^...commitB) so it was easy to take these fresh commits and put them on the branch I already made to test it the first time. Everything seemed to test out fine (nice!), so I'll just merge my branch where I tested this and close this PR. Definitely fetch and rebase your master before any future features though so we're back in sync

MarioKusek commented 5 years ago

That is excellent news.

When you merge it I will rebase my master and go from there to implement issue #29

MarioKusek commented 5 years ago

Thanks a lot!