sailfishos-patches / patchmanager

Patchmanager for SailfishOS
https://openrepos.net/content/patchmanager/patchmanager
Other
21 stars 22 forks source link

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

Open robang74 opened 1 year ago

robang74 commented 1 year ago

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.

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

nephros commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@nephros @nephros @Olf0 fyi

Olf0 commented 1 year ago

@b100dian was missed.

Olf0 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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.