Added a lot of comments

[marcelloh]

and made it a "go modules" version. (did some more in my latest master, but I'm not sure about the last parts, so if you could do them....)

[marcelloh]

why isn't this merged? I did my thing, so it is easy for you to do yours.

[rubenv]

Well, you haven't exactly made my life easy.

Remember what I said in #148?

Do keep the pull requests small and focused, 10 PRs of 10 lines have a much higher chance of getting in than 1 PR of 100 lines.

Instead we now have a PR of 18 commits and over 400 lines of changes.

To make matters worse, there's stuff in there that doesn't belong together, I see code comments, a Makefile, go modules. None of these should have gone together in one PR.

Also, as hinted before, most of the warnings in golint are just pointless noise. so there's very little value in them.

There's also a lot of changes that are questionable:

/*
Migration is a method receiver.
*/
type Migration struct {

What does that even mean and what is the value in that comment?

When contributing to a project, please be mindful of the existing code style. Block comments (/* ... */) aren't used anywhere in the project, we use double-slash style (// ...).

So merging this as-is would bring in a lot of inconsistencies, unwanted changes and things that don't really make sense (at least to me, happy to discuss).

Having both the flu and a baby in the house meanse I haven't gotten the time yet to look at it in detail and pick out the good parts (they're certainly there!), apologies for that. It's just that by lumping it all into one big pile you've made it super hard for the maintainer to work with it and as such I haven't found the energy to dive into it.

[marcelloh]

You can use a merge tool to merge the good parts ;-) But the make file comes in handy btw. And the modules too. (it's perhaps new to you, but not so new these days in the go dev world.) Anyways... see & take what you like.

Op do 10 okt. 2019 om 09:33 schreef Ruben Vermeersch < notifications@github.com>:

Well, you haven't exactly made my life easy.

Remember what I said in #148 https://github.com/rubenv/sql-migrate/issues/148?

Do keep the pull requests small and focused, 10 PRs of 10 lines have a much higher chance of getting in than 1 PR of 100 lines.

Instead we now have a PR of 18 commits and over 400 lines of changes.

To make matters worse, there's stuff in there that doesn't belong together, I see code comments, a Makefile, go modules. None of these should have gone together in one PR.

Also, as hinted before, most of the warnings in golint are just pointless noise. so there's very little value in them.

There's also a lot of changes that are questionable:

/Migration is a method receiver./type Migration struct {

What does that even mean and what is the value in that comment?

When contributing to a project, please be mindful of the existing code style. Block comments (/ ... /) aren't used anywhere in the project, we use double-slash style (// ...).

So merging this as-is would bring in a lot of inconsistencies, unwanted changes and things that don't really make sense (at least to me, happy to discuss).

Having both the flu and a baby in the house meanse I haven't gotten the time yet to look at it in detail and pick out the good parts (they're certainly there!), apologies for that. It's just that by lumping it all into one big pile you've made it super hard for the maintainer to work with it and as such I haven't found the energy to dive into it.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/rubenv/sql-migrate/pull/149?email_source=notifications&email_token=AEFOQERP542DRFZCYDKIJSDQN3LDFA5CNFSM4I32PNO2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEA3GGPI#issuecomment-540435261, or unsubscribe https://github.com/notifications/unsubscribe-auth/AEFOQEUWBFSDPN5LFLPJIATQN3LDFANCNFSM4I32PNOQ .

[rubenv]

It's not about whether a Makefile is handy, or that modules are new to me (don't make assumptions, they most certainly aren't). It's that they don't belong together in a pull request that's supposed to be about comments.

Remember that most open-source maintainers do this on spare time, have the courtesy of making their lives easy. One change per PR is common etiquette and facilitates discussion.