Skip to content

Combined pull requests (status, BOM handling, sequential ordering, version conflict detection, etc) #28

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

Closed
wants to merge 75 commits into from

Conversation

Mazrick
Copy link

@Mazrick Mazrick commented Sep 24, 2015

I wanted all of the features available in the pull requests:

So, I started from the most current master, and merged those pulls into this branch.

I also:

  • fixed a BOM issue
    • saving a script in windows with UTF-8 characters, fails to migrate when a BOM is inserted
  • Fixed/added version conflict detection
  • Added change filename validation configuration option
  • Added sequence number padding configuration option for filenames

I have fully tested locally with updated unit tests.

There were some dependency changes made to the pom.xml on some of the pull requests, which I am not fully informed about, so if there are any pieces which should first be reverted or changed, please feel free to comment and enlighten me!

simotripo and others added 30 commits February 11, 2013 21:36
 * SCM coordinates
 * issue management
 * CI Management
 * distribution management
…efault for all of the templates. Now both "--//" and "-- //" are acceptable. "-- //" is a more standard way to use comments in standard sql. Now editors should not complain about our marker comments.
WalterPhillips and others added 19 commits March 10, 2015 09:39
Add some text in the status description field to indicate if a script file is
not found in the file system. This can happen, for example, when running
status on a database when the local environment is on a branch that does not
include one of the migrations previously applied to the database.
…ion to the New command to support testing and add to functionality
@Mazrick Mazrick mentioned this pull request Sep 25, 2015
Crispin Botticelli added 2 commits September 25, 2015 12:44
@hazendaz
Copy link
Member

hazendaz commented Apr 9, 2016

@Mazrick Can you rebase and squash your commits?

@harawata
Copy link
Member

@Mazrick , Thank you for your contribution!
But combining multiple PRs does not seem to be a good idea to me.

Please use the GitHub's reaction feature to up/down vote the PRs instead.
This is very important for us to decide which PR to merge. :-)

@harawata harawata closed this Apr 10, 2016
@Mazrick
Copy link
Author

Mazrick commented Apr 21, 2016

@harawata I tend to agree with you, but we are forced to do things we don't really like to when the maintainer lag is literally Mar 9, 2015 - Apr, 9, 2016.
I'll consider going through the work of re-basing and re-contributing all of the PRs separately.

@hazendaz
Copy link
Member

We are trying to improve our support so lagging goes away. If you can get us something it will see a release very soon.

Sent from Outlook Mobile

On Thu, Apr 21, 2016 at 9:25 AM -0700, "Crispin Botticelli" [email protected] wrote:

@harawata I tend to agree with you, but we are forced to do things we don't really like to when the maintainer lag is literally Mar 9, 2015 - Apr, 9, 2016.
I'll consider going through the work of re-basing and re-contributing all of the PRs separately.


You are receiving this because you commented.
Reply to this email directly or view it on GitHub:
#28 (comment)

@harawata
Copy link
Member

harawata commented Apr 21, 2016

Thank you for your understanding and contribution, @Mazrick !

We have already merged #29 and implemented #16 (in a different way. see #14).
Feedback for these changes is also welcome :-)

Regards,
Iwao

@emacarron
Copy link
Member

@Mazrick considering how much you have paid for the tool I suppose you can imagine that we work here in our spare time and just for fun so I am also sorry for the lag but this is how ths works.

@Mazrick
Copy link
Author

Mazrick commented Apr 22, 2016

@emacarron, I totally appreciate the effort you guys have put into this tool! It does a lot of what I needed in a very straight forward way!
I only paid weeks of time vs. the months of effort evident in this solution! I'm not trying to complain about the lag, just point out in response to @harawata 's comment that I was forced into combining the PRs myself locally, which I didn't want to do. I was simply boxed into a corner at the time.

At this point development has diverged a bit, as evidenced by the fact that some of the fixes and merges are built upon each other, and some of the code was merged into the base and some of my changes are not. So I'm thinking about separating them into staged pull requests that build on each other. I'll endeavor to submit only one PR at a time, then only after it is accepted, I'll submit the next, if it does indeed build on the previous steps.

Thank You!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants