Skip to content
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

Restore RAR 6.0 compatibility #4154

Closed
mc-butler opened this issue Dec 10, 2020 · 11 comments
Closed

Restore RAR 6.0 compatibility #4154

mc-butler opened this issue Dec 10, 2020 · 11 comments
Assignees
Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress
Milestone

Comments

@mc-butler
Copy link

Important

This issue was migrated from Trac:

Origin https://midnight-commander.org/ticket/4154
Reporter birdie (aros@….com)

Now that RAR 6.0 has been released this code breaks and uses the RAR 4 handler for RAR 6 archives. Adjust the comparison to use the RAR 5 handler for all future releases by default now that rar/unrar 4.0 is getting out of use.

#158

@mc-butler
Copy link
Author

Changed by birdie (aros@….com) on Dec 10, 2020 at 18:18 UTC (comment 1)

It's a very simple fix, please merge ASAP.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Dec 10, 2020 at 18:32 UTC (comment 2)

We don't use pull requests. Please remember that.

@mc-butler
Copy link
Author

Changed by birdie (aros@….com) on Dec 10, 2020 at 18:44 UTC (comment 2.3)

Replying to andrew_b:

We don't use pull requests. Please remember that.

Please merge however you want. :-) The fix is provided.

@mc-butler
Copy link
Author

Changed by birdie (aros@….com) on Dec 10, 2020 at 18:48 UTC (comment 4)

Sometimes it all looks a little bit f---ed up. An issue is reported, a fix is provided and instead of merging it and maybe, God forbid, thanking the reporter, you say "We don't use pull requests". This surely looks a little bit like "F--- off, we're OK with our bugs and our snail's pace fixes".

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Dec 10, 2020 at 18:55 UTC (comment 3.5)

Replying to birdie:

Replying to andrew_b:

We don't use pull requests. Please remember that.

Please merge however you want. :-) The fix is provided.

Such fix without tests is incomplete.

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Dec 11, 2020 at 12:01 UTC (comment 6)

This surely looks a little bit like "F--- off, we're OK with our bugs and our snail's pace fixes".

It might indeed look this way from the your point of view.

From the maintainers point of view, it looks quite different though:

  1. Someone comes around, and doesn't give a shit about what processes the maintainers use. (Okay, fair enough - at least you were so nice as to open a ticket to point to your pull request - most don't even care to invest that much effort.)
  2. You quickly fix your own problem with smallest possible fix, of course, without any visible tests (there are VFS tests, which you should have extended!) and not even background information of how you tested it and what the change is supposed to do / why it is implemented this way.
  3. Now apparently we have to do 20x more work than you invested, and URGENTLY - because if we just merge, release without doing the work and it doesn't work for others, we are going to get a shitload of aggressive reports "WHY DID YOU BREAK MY RAR!!!11", which always happen when we miss some mistake in user patch.
  4. If we don't do it URGENTLY, then you come again and went your unwarranted frustration, thanks for that. In fact, whatever we do, we lose.

So actually, to me it rather sounds like "F--- off maintainers, you are stupid assholes and I'm the king on the white horse".

@mc-butler
Copy link
Author

Changed by birdie (aros@….com) on Dec 11, 2020 at 14:31 UTC (comment 7)

  • Version master deleted
  • Milestone 4.8.26 deleted
  • Priority changed from major to trivial

I get it, the fix is so effing complicated it requires unit tests and sh*t even though you were totally OK with not having any unit tests for this code in the past. So, not only I have to report a bug, I also have to write unit tests, learn your workflow and send you an appropriate patch for four effing modified characters. I will do my best not to bother you any longer with stupid patches and stupid bug reports. You definitely need neither.

Please close this bug report as invalid.

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 13, 2021 at 11:25 UTC (comment 8)

  • Milestone set to 4.8.27
  • Status changed from new to accepted
  • Version set to master
  • Owner set to andrew_b
  • Priority changed from trivial to major
  • Branch state changed from no branch to on review

Branch: 4154_rar6
[84b435fa39286f25cb2d3a85db94b023a1d89092]

@mc-butler
Copy link
Author

Changed by zaytsev (@zyv) on Feb 13, 2021 at 13:49 UTC (comment 9)

  • Votes set to zaytsev
  • Branch state changed from on review to approved

Thanks Andrew!

P.S. What a drama...

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 13, 2021 at 13:54 UTC (comment 10)

  • Status changed from accepted to testing
  • Resolution set to fixed
  • Votes changed from zaytsev to committed-master
  • Branch state changed from approved to merged

Merged to master: [b0ed56e].

@mc-butler
Copy link
Author

Changed by andrew_b (@aborodin) on Feb 13, 2021 at 13:56 UTC (comment 11)

  • Status changed from testing to closed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: vfs Virtual File System support prio: medium Has the potential to affect progress
Development

No branches or pull requests

2 participants