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

Cleaning, hardening, applying common style, testing ... then adding new functions #93

Open
1 of 7 tasks
igorpecovnik opened this issue Sep 22, 2024 · 6 comments
Open
1 of 7 tasks

Comments

@igorpecovnik
Copy link
Member

igorpecovnik commented Sep 22, 2024

Which feature would you like to have?

  • indentation is not standardised. Some place have no spaces, some have 2 spaces, some 3, some 4 and in some place there are tabs.
  • there are multiple functions that are added that does the same thing. For example: kernel headers install
  • functions are not defined in their corresponding modules sh file.
  • there are spelling mistakes, function descriptions that were just copied over from another function and were never modified
  • non existing naming convention used for naming functions and variables. Sometimes function names started in caps some times in lowercase
  • in some places script is called configng, some places its called armbian-config, some places its called armbian-configng. So there is no standard name being used.

Funding

@ThomasKaiser
Copy link

@Tearran
Copy link
Member

Tearran commented Sep 23, 2024

Congratulations, only two stupid temp file vulnerabilities:

@ThomasKaiser Not sure if this was intended to be sarcastic, but setting aside the unnecessary language, pointing out a potential vulnerability without offering an alternative solution isn’t constructive. You're welcome to submit a pull request with your suggested changes—I’d be happy to review it.

And nowhere PATH is defined, isn't it?

It seems you're implying the shebang is not sufficient for the production environment. However, explicitly setting a PATH seems excessive for the development context, especially since it’s not a requirement of the project.

@ThomasKaiser
Copy link

setting aside the unnecessary language, pointing out a potential vulnerability without offering an alternative solution isn’t constructive

OMG, you must be from the USA, right?

Do yourself a favour and do a web search for 'temp file vulnerability' or simply a man mktemp.

However, explicitly setting a PATH seems excessive for the development context, especially since it’s not a requirement of the project.

Stunning... this level of non-understanding is new to me. But it's clearly 'Armbian style' :)

@ThomasKaiser
Copy link

OMG, greedy Igor doing Igor-things armbian/build#7291

Not understanding much about USB hassles (from 3 years ago), he chose to add some vendor/product IDs to a script he doesn't understand. Is "Armbian" about to be sold to someone else?

@Tearran
Copy link
Member

Tearran commented Sep 24, 2024

@ThomasKaiser It appears that constructive communication is not your priority. If I am mistaken, I encourage you to review our CODE_OF_CONDUCT and its associated guidelines: https://github.com/armbian/build?tab=coc-ov-file.

@igorpecovnik
Copy link
Member Author

Congratulations, only two stupid temp file vulnerabilities:

Several functions has been copy pasted and are waiting for review and rework. Not everything was written from scratch.

Development is great fun but our time is limited. This is the reason why things are sometimes done sloppy. You are welcome to join the enjoyment and make a PR.

Thank you for letting us know.

Not understanding much about USB hassles (from 3 years ago), he chose to add some vendor/product IDs to a script he doesn't understand. Is "Armbian" about to be sold to someone else?

My motive was nothing else but trying to quick-close some old issues on a project we are about to make as read-only archive soon.

Amazing what kind of conclusions can be extracted from ,0x04e8:0x61f5:u :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants