search

frioux / DBIx-Class-DeploymentHandler

https://metacpan.org/pod/DBIx::Class::DeploymentHandler
20 stars 25 forks source link

keep DH::DM::S:T from creating files on windows that are rejected on linux #46

Closed wchristian closed 5 years ago

wchristian commented 8 years ago

Linux MySQL really doesn't like the upgrade files with \r\n in them, this patch prevents that.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-6.1%) to 86.19% when pulling bf5ed624cabec477c915886fa1027ebef67e3941 on wchristian:win32_fixes into 9f877d3f862eb15c9c27e6e661ce2a4c6929db15 on frioux:master.

frioux commented 8 years ago

Any chance you could write a test for this?

wchristian commented 8 years ago

Will do.

frioux commented 7 years ago

Would love to merge this, any updates?

sent from a rotary phone, pardon my brevity

On May 4, 2016 1:18 AM, "Christian Walde (Mithaldu)" < notifications@github.com> wrote:

Will do.

— You are receiving this because you commented. Reply to this email directly or view it on GitHub https://github.com/frioux/DBIx-Class-DeploymentHandler/pull/46#issuecomment-216779308

frioux commented 7 years ago

It's been a year; closing this.

wchristian commented 7 years ago

That is the most ill-considered reaction you could possibly have here. Sorry i have not had the time to write tests. However rejecting the code for lack of tests is 100% counter-productive, since it's absolutely trivial and obvious, and entirely needed.

frioux commented 7 years ago

So write them?

-- Sent from a telephone. Pardon my brevity.

On May 26, 2017 1:38 AM, "Christian Walde (Mithaldu)" < notifications@github.com> wrote:

That is the most ill-considered reaction you could possibly have here. Sorry i have not had the time to write tests. However rejecting the code for lack of tests is 100% counter-productive, since it's absolutely trivial and obvious, and entirely needed.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-DeploymentHandler/pull/46#issuecomment-304225677, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAf46kMwo6wdBmSp5as95J0O9VafNP7ks5r9o9zgaJpZM4IV4kL .

wchristian commented 7 years ago

I intend to, but you could also be less of an ass about this by poking for an update before closing, or at least pulling in the functional bits even if there are no tests yet. I've not forgotten about this at all and see the tab at least once a week. I've just always had more important things calling for me. All i've done so far is reported an actual problem, provided a fix, and failed to provide tests. Your response was to reject the entire bug just because there are no tests yet, and frankly i'm pretty pissed that such a thing is a good enough excuse for you to continue releasing software with known bugs.

frioux commented 7 years ago

I'm not going the read something calling me an ass when I provide free software to you. Feel free to try again.

-- Sent from a telephone. Pardon my brevity.

On May 26, 2017 7:47 AM, "Christian Walde (Mithaldu)" < notifications@github.com> wrote:

I intend to, but you could also be less of an ass about this by poking for an update before closing, or at least pulling in the functional bits even if there are no tests yet. I've not forgotten about this at all and see the tab at least once a week. I've just always had more important things calling for me. All i've done so far is reported an actual problem, provided a fix, and failed to provide tests. Your response was to reject the entire bug just because there are no tests yet, and frankly i'm pretty pissed that such a thing is a good enough excuse for you to continue releasing software with known bugs.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-DeploymentHandler/pull/46#issuecomment-304302393, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAf4_HVVmG9-Rq1hnzWZi_pSDQwyjnZks5r9uYCgaJpZM4IV4kL .

wchristian commented 7 years ago

You closing this bug after a year of releasing software with a known bug for which you had been given a fix was enough of a slap in the face that i don't give a lot of weight to the argument you tried to advance there.

It doesn't become any less of a bug if you close this.

How about you go first with treating people who provide free fixes for mistakes with respect.

frioux commented 7 years ago

Ah I understand the problem. I agree that the bug should remain open, but I think an incomplete pull request is not beneficial. If you want to open a bug and link to this pr I'd leave the bug open.

Please don't feel like me not fixing bugs is a slap in your face. I don't understand what the real bug here is and without a test I am unwilling to adopt the code you have written. Many times I have been given code that fixes bugs without tests, only to find that the changes make unrelated things worse without any clear benefit. I am not saying that's the situation here, but it is my policy. -- Sent from a telephone. Pardon my brevity.

On May 26, 2017 8:24 AM, "Christian Walde (Mithaldu)" < notifications@github.com> wrote:

You closing this bug after a year of releasing software with a known bug for which you had been given a fix was enough of a slap in the face that i don't give a lot of weight to the argument you tried to advance there.

It doesn't become any less of a bug if you close this.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-DeploymentHandler/pull/46#issuecomment-304311788, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAf41RcYVlaKp5dsjhPiUAG8Hww6M1Aks5r9u68gaJpZM4IV4kL .

wchristian commented 7 years ago

The not fixing is not the slap, for me it was only the closing of the report, which implied there is no bug.

I created a corresponding issue. Maybe the explanation therein also helps understand the issue better.

frioux commented 7 years ago

Thanks Christian!

-- Sent from a telephone. Pardon my brevity.

On May 26, 2017 8:59 AM, "Christian Walde (Mithaldu)" < notifications@github.com> wrote:

The not fixing is not the slap, for me it was only the closing of the report, which implies there is no bug.

I created a corresponding issue. Maybe the explanation therein also helps understand the issue better.

— You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub https://github.com/frioux/DBIx-Class-DeploymentHandler/pull/46#issuecomment-304320490, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAf49-NBSqfjIhDEDxmqQmWfivEpmHNks5r9vbYgaJpZM4IV4kL .

mohawk2 commented 5 years ago

The strategy here (https://github.com/Perl-Toolchain-Gang/ExtUtils-Manifest/pull/3/files), together with using AppVeyor, should solve the testing problem.

@wchristian Feel like adding a .appveyor.yml to the PR?

@frioux Could you enable AppVeyor on this repo?

frioux commented 5 years ago

Sure, do you have a link on how?

On Thu, Mar 07, 2019 at 12:00:19AM -0800, mohawk2 wrote:

The strategy here (https://github.com/Perl-Toolchain-Gang/ExtUtils-Manifest/pull/3/files), together with using AppVeyor, should solve the testing problem.

@wchristian Feel like adding a .appveyor.yml to the PR?

@frioux Could you enable AppVeyor on this repo?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/frioux/DBIx-Class-DeploymentHandler/pull/46#issuecomment-470424537

-- fREW Schmidt https://blog.afoolishmanifesto.com

wchristian commented 5 years ago

Honestly at this point just leave me out of it. The bug is real and obvious and utterly trivial and requires no higher level of comprehension, the fix is the same, and it's been 3 years. The difference between writing text-mode files and binary-mode files are well-known and documented even in perl core.

https://perldoc.perl.org/perlport.html#Newlines

I'm unsubscribing, don't @ me. Bye.

mohawk2 commented 5 years ago

Let this stand as an example of how not to report (and probably respond to) bugs in software.

@frioux https://www.appveyor.com/docs/ - it's really pretty straightforward. Based on the above, I won't be putting any special effort into resolving this.

frioux commented 5 years ago

Ok, enabled, but yes, I (clearly) long ago gave up on this.