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.

Sign up for GitHub

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

Jump to bottom

refactor: apt-get with nala and hardcoded package managers #422

Merged
merged 5 commits into from
Sep 18, 2024
Merged

refactor: apt-get with nala and hardcoded package managers #422

merged 5 commits into from
Sep 18, 2024

Conversation

jeevithakannan2
Copy link
Contributor

@jeevithakannan2 jeevithakannan2 commented Sep 16, 2024 •
edited
Loading

Pull Request

Title

Use nala , replace hardcoded package managers, add missing checkEnv and checkEscalationTool, implement posix recommended way of using variables

Type of Change

  • Refactoring

Description

  • Position nala in front in common-script so it would use nala as package manager if found
  • Add nala in some missing scripts
  • Replace hardcoded package managers with PACKAGER variable
  • Added checkEnv in numlock for consistency among scripts
  • Added checkEscalationTool in monitor control for consistency among scripts
  • Gaming setup is not included in this PR as it is getting a rework in Refactor gaming-setup.sh #380

Issue related to PR

Testing

  • Tested in debian12 with nala installed works as expected.
  • Tested other modified scripts in arch works as expected.

Additional Information

Warning

#440 #442 #443 are implemented after this PR. Check those PRs with mine and decide which one to merge

Caution

The implementation of #440 is wrong
Replacing apt-get with apt is not recommended when using in scripts.
Only if we switch nala and apt-get places in common script it will check for nala first if found it will choose nala as the package manager.
If not it will check for apt-get. On his implementation it will use apt itself as it checks for apt first.

Checklist

  • My code adheres to the coding and style guidelines of the project.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • My changes generate no errors/warnings/merge conflicts.

nnyyxxxx

This comment was marked as outdated.

Sign in to view

This was referenced Sep 17, 2024
Merged
Closed
Closed
Closed
Closed
@nnyyxxxx
Copy link
Contributor

nnyyxxxx commented Sep 17, 2024 •
edited
Loading

And again you're literally just putting my changes into this one @jeevithakannan2 as seen with the commits before 100fc58 this one was incomplete... You shouldn't lie..

#440 #442 #443 are implemented after this PR with half of the my changes only. It is not completely finished. Check those two PRs with mine and decide which one to merge

@jeevithakannan2
Copy link
Contributor Author

jeevithakannan2 commented Sep 17, 2024 •
edited
Loading

Go on and spam in other people PR's after copying them

@nnyyxxxx
Copy link
Contributor

nnyyxxxx commented Sep 17, 2024 •
edited
Loading

Go on and spam in other people PR's after copying them

  1. Did not copy you, 2. I'm competing with this PR, 3. Your PR was incomplete when I had made the other PRs, 4. The blatant lying is ridiculous when we have a commit that explains everything... Compare the dates of the creation if you want: 100fc58 before the changes: 61cddeb

@jeevithakannan2
Copy link
Contributor Author

jeevithakannan2 commented Sep 17, 2024 •
edited
Loading

Go on and spam in other people PR's after copying them

1. Did not copy you, 2. I'm competing with this PR, 3. Your PR was incomplete when I had made the other PRs, 4. The blatant lying is ridiculous when we have a commit that explains everything... Compare the dates of the creation if you want: [100fc58](https://github.com/ChrisTitusTech/linutil/commit/100fc58daa91b127187497f0061922786747db0d) before the changes: [61cddeb](https://github.com/ChrisTitusTech/linutil/commit/61cddeb2566f0c874c15428986381832203f3591)
  1. If you didn't copy then how come I copied you? 2. You could have mentioned some places are missing rather than creating 3 extra PRs 3. I didn't lie I don't remember saying I have covered all the scripts. 4. You had some places not covered before this commit 4404169 So that means you were lying that you covered all the scripts??

@nnyyxxxx
Copy link
Contributor

Go on and spam in other people PR's after copying them

1. Did not copy you, 2. I'm competing with this PR, 3. Your PR was incomplete when I had made the other PRs, 4. The blatant lying is ridiculous when we have a commit that explains everything... Compare the dates of the creation if you want: [100fc58](https://github.com/ChrisTitusTech/linutil/commit/100fc58daa91b127187497f0061922786747db0d) before the changes: [61cddeb](https://github.com/ChrisTitusTech/linutil/commit/61cddeb2566f0c874c15428986381832203f3591)

1. If you didn't copy then how come I copied you? 2. You could have mentioned some places are missing rather than creating 3 extra PRs 3. I didn't lie I don't remember saying I have covered all the scripts. 4. You had some places not covered before this commit [4404169](https://github.com/ChrisTitusTech/linutil/commit/4404169e82ff774aa9d39c9f2c961e6900723c0a) So that means you were lying that you covered all the scripts??

No, not exactly, I had missed those but my original implementation was correct, and BTW you shouldn't blatantly take my semi-complete fix and put it into a half baked PR.. And then start an argument over it after taking everything that was in my PR that fixed everything in this.. Here are the commits before & after you blatantly lied: before: 61cddeb after: 100fc58

@jeevithakannan2
Copy link
Contributor Author

Done with you do as you wish

@nnyyxxxx
Copy link
Contributor

You're still missing some stuff that is correctly implemented in my 3 PRs, and you're modifying the aurhelpers when that wont be needed with #382 . Lets stop arguing over this nonsense... Just close this as its incomplete. @jeevithakannan2

@jeevithakannan2
Copy link
Contributor Author

I don't understand what you're calling incorrect in my PR and correct in yours??

@adamperkowski
Copy link
Collaborator

I don't understand what you're calling incorrect in my PR and correct in yours??

  1. "Replacing apt-get with apt is not recommended when using in scripts." - apt overall has more features than apt-get. apt-get is "deprecated" in some distros. Why do you think it's not recommended? I recommend using apt.
  2. "On his implementation it will use apt itself as it checks for apt first." - And what's wrong with this?
  3. "If you didn't copy then how come I copied you?" - ....??? You copied changes nyx made in their PRs without crediting nyx. 100fc58 & 6a91e7d (other commits in Change apt-get's & use nala along with apt #440 , Fix issues with packager #442 & Fix missing escalation tool #443 as well). Check timestamps.

Is this enough?

@nnyyxxxx
Copy link
Contributor

@jeevithakannan2 Please man stop arguing with us, we have proved you wrong many times over. Just close this...

@adamperkowski
Copy link
Collaborator

I think #440 #442 #443 approach this better. #422 is clearly copying the changes from them into his own implementation.

@ChrisTitusTech ChrisTitusTech merged commit 88468b1 into ChrisTitusTech:main Sep 18, 2024
2 checks passed
@ChrisTitusTech ChrisTitusTech added the bug Something isn't working label Sep 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nnyyxxxx nnyyxxxx nnyyxxxx approved these changes

@adamperkowski adamperkowski adamperkowski approved these changes

@ChrisTitusTech ChrisTitusTech ChrisTitusTech approved these changes

Assignees
No one assigned
Labels
bug Something isn't working refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jeevithakannan2 @nnyyxxxx @adamperkowski @ChrisTitusTech