saschakiefer / generator-openui5

yeoman generator for OpenUI5 applications and assets
Other
64 stars 17 forks source link

Livereload prompt feature #42

Closed js1972 closed 10 years ago

js1972 commented 10 years ago

Added a prompt (y/n) for live-reload in grunt. Minor fix of the Tiles app in case the user removes the mock Northwind service so that it uses the correct localhost port. Minor comment changes in the Tiles app as well...

saschakiefer commented 10 years ago

It would be great if you could add the closes statement in the footer of a commit message in future commits.

e.g.: closes #42

Then when committing, github automatically references the commit to the issue it belongs to. I think this is a great way to easily find out where the changes are done, when you're starting from the issue and want to see what changed.

js1972 commented 10 years ago

You specifically want this in the footer? I have done it a few times where I've remembered but as my repo is a fork of yours I have an independent issue list. I'll stop using that and only use the master list (yours).

On 4 February 2014 15:12, saschakiefer notifications@github.com wrote:

It would be great if you could add the closes statement in the footer of a commit message.

e.g.: closes #42https://github.com/saschakiefer/generator-openui5/pull/42

Then when committing, github automatically references the commit to the issue it belongs to. I think this is a great way to easily find out where the changes are done, when you're starting from the issue and want to see what changed.

Reply to this email directly or view it on GitHubhttps://github.com/saschakiefer/generator-openui5/pull/42#issuecomment-34035808 .

saschakiefer commented 10 years ago

Hmm, good point. I didn't think about your fork. Maybe we can try it in one of your future commits with the master issue number (I don't know, what github does if you commit on your fork closing issues that are not there). I'd expect it to be reassigned, since I checkout and do a merge commit. But I think we should try that first, as long as it's not too much trouble for you matching the numbers.

As I said, I just find it a very useful feature. If I imagine one of our future users (which we hopefully have) reports an issue which he wants to track, he can immediately see what and when we committed that. It also makes my live easier when closing messages to see, that a fix was committed.

js1972 commented 10 years ago

Probably simpler if I stop doing any issues in my fork and only use the master repo for issues. Then I won't have any confusion and will add the issues url's in the footer of commits. ;-)

On Tue, Feb 4, 2014 at 3:21 PM, saschakiefer notifications@github.com wrote:

Hmm, good point. I didn't think about your fork. Maybe we can try it in one of your future commits with the master issue number (I don't know, what github does if you commit on your fork closing issues that are not there). I'd expect it to be reassigned, since I checkout and do a merge commit. But I think we should try that first, as long as it's not too much trouble for you matching the numbers.

As I said, I just find it a very useful feature. If I imagine one of our future users (which we hopefully have) reports an issue which he wants to track, he can immediately see what and when we committed that. It also makes my live easier when closing messages to see, that a fix was committed.

Reply to this email directly or view it on GitHub: https://github.com/saschakiefer/generator-openui5/pull/42#issuecomment-34036126

js1972 commented 10 years ago

Just added a commit to the livereload branch in the current pull-request to fix issue https://github.com/saschakiefer/generator-openui5/issues/44. I added "Closes https://github.com/saschakiefer/generator-openui5/issues/44" in the footer and it has updated the issue but hasn't actually closed it. Maybe its smart enough to close it when the pull-request is merged. I'm not sure whether to laugh or cry though - that refactoring work I did to use extend instead of inherits was perfectly fine. The bug was introduced many many commits ago when the prompts were broken out into their own functions instead of one big one.

saschakiefer commented 10 years ago

I didn't manage to automatically close the issue while commit, but at least you get the reference to the commit in the issue when you commit it (which also worked for #44)

With respect to the introduced bug. Look at the bright side. At least we learned sth. 1) we depserately need content tests 2) the inheritance works in principle

I'm going to completely remove the comments with the next commit (have to modify index.js anyways to fix #19)