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

[Suggestion] the pm_apply and pm_unapply can be unified togheter #444

Open
robang74 opened this issue Jul 1, 2023 · 9 comments
Open

[Suggestion] the pm_apply and pm_unapply can be unified togheter #444

robang74 opened this issue Jul 1, 2023 · 9 comments
Labels
feature request feature requests, suggestions, ideas etc.

Comments

@robang74
Copy link

robang74 commented Jul 1, 2023

DESCRIPTION

I have unified the pm_apply and pm_unapply shells script in a single one pm_patch.env because most of the code was redundant.

  • pm_apply does source pm_patch.env apply "$@"
  • pm_unapply does source pm_patch.env unapply "$@"

This would help to maintain such shell script code in the future.

ADDITIONAL INFORMATION

Check this branch

https://github.com/sailfishos-patches/patchmanager/compare/master...robang74:patchmanager:devel?diff=unified

@robang74 robang74 added the feature request feature requests, suggestions, ideas etc. label Jul 1, 2023
@nephros
Copy link
Contributor

nephros commented Jul 3, 2023

This indeed looks like a sound idea.

However your devel branch at the moment contains changes unrelated to this.

If/When your unification work is complete, please submit a PR containing just the unifying changes so we can review and discuss in detail.

Thanks.

@nephros
Copy link
Contributor

nephros commented Jul 3, 2023

What I mean is a PR with just movs from the two scripts to the unified three
scripts, leaving all previous functionality unchanged.

After this is merged, we can work on PRs on top of this to include any
improvements or bug fixes.

@CODeRUS
Copy link
Contributor

CODeRUS commented Jul 21, 2023

Please take care to fix the code issues reported by this person, but create your own changes. no commits authored by this person are allowed to be merged in this repo.

@CODeRUS
Copy link
Contributor

CODeRUS commented Jul 21, 2023

@nephros @nephros @Olf0 fyi

@Olf0
Copy link
Contributor

Olf0 commented Jul 22, 2023

@b100dian was missed.

@Olf0
Copy link
Contributor

Olf0 commented Jul 22, 2023

Please take care to fix the code issues reported by this person, …

@CODeRUS, I can assure you that we already do. The major reasons are, that both, his statements and code, are always very convoluted and contain flaws (linguistic, logic or simply understanding). We have had a lengthy discussion how to deal with this and his behaviour in general at FSO, hence I think we have pondered enough to draw some conclusions.

Still I have some issues with the rest of your statement. Can you please explain a bit and maybe reference, what triggered you to make these bold statements.

@CODeRUS
Copy link
Contributor

CODeRUS commented Jul 24, 2023

@Olf0 thank you. It's about generally unfriendly behavior towards the community, ignoring basic things like code of conduct, etc. You can have a look at the sfos forum.

@Olf0
Copy link
Contributor

Olf0 commented Jul 24, 2023

@CODeRUS, all agreed, ignorance, arrogance and hubris are obviously three of his strong points. But I believe one should avoid to think and act on a tit-for-tat basis, because that makes one's own behaviour not any better; also, we are all human, all have weak points, and I do understand that (to a far lesser extent, I hope) others have occasionally perceived me as arrogant, too (IIRC that may be applicable to you as well).

Ultimately, I strongly believe we should judge any contribution primarily on technical grounds, not if it came from person X. Nephros has stated one essential of the usual requirements for a PR to be reviewed (here: one thing at a time, i.e., de-convolute the current PR by splitting it), there has been no reply in three weeks, and extrapolating from other interactions, there will be none. But if he submits his changesets one by one (i.e., in separate PRs), I strongly believe we should handle them as usual: Review them, maybe request changes and finally merge them, if they make sense and are technically sound.

@robang74
Copy link
Author

robang74 commented Jul 24, 2023

@Olf0 wrote

all agreed, ignorance, arrogance and hubris are obviously three of his strong points.
[...]
Ultimately, I strongly believe we should judge any contribution primarily on technical grounds, not if it came from person X.

Looks like that hypocrisy and quick judgmental attitude are not a problem of yours or I am earning a new fan? ;-)

Starting to use github as is supposed for, it can be a neutral good start. IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request feature requests, suggestions, ideas etc.
Projects
None yet
Development

No branches or pull requests

4 participants