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

Very little fix of synthaxe #98

Open
wants to merge 6 commits into
base: master
from

Conversation

@liberodark
Copy link

commented Jun 28, 2019

Problem

  • Description of why you made this PR
    Have not good synthaxe is not very easy to make a 100% proper bash
    The idea is make a more better bash script for help packager to make better bash

Solution

  • And how do you fix that problem

PR Status

  • Code finished.
  • Tested with Package_check.
  • Fix or enhancement tested.
  • Upgrade from last version tested.
  • Can be reviewed and tested.

Package_check results


If you have access to App Continuous Integration for packagers you can provide a link to the package_check results like below, replacing '-NUM-' in this link by the PR number and USERNAME by your username on the ci-apps-dev. Or you provide a screenshot or a pastebin of the results

Build Status

liberodark added some commits Jun 28, 2019

@liberodark liberodark referenced this pull request Jun 28, 2019
0 of 5 tasks complete
@maniackcrudelis

This comment has been minimized.

Copy link
Contributor

commented Jun 29, 2019

You don't need that many double quotes everywhere !
Double quotes should be used when needed, not everywhere to be sure it would works. Especially since it won't !

Double quotes have to be used when

  • You expect a variable with special characters or spaces.
  • You use test -n or test -z
  • You use a string comparison.

Otherwise, double quotes shouldn't be used, it may raise errors, especially if you use them for digit comparisons and can failed cli command if not used correctly.
Putting double quotes everywhere is not a good solution, you have to know and understand where and why use them.

@liberodark

This comment has been minimized.

Copy link
Author

commented Jun 30, 2019

Now this bash is not clean without double quote and no have make double quote everywhere.
An so I also know how to use it but it's much cleaner for a more compatible code.
Make tests you'll see that I'm right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.