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

Feat styleguide #62

Merged
merged 5 commits into from
Feb 15, 2016
Merged

Feat styleguide #62

merged 5 commits into from
Feb 15, 2016

Conversation

bastelfreak
Copy link
Member

I would like to discuss a few guidelines for our shell code. This stuff is based on the questions occurred during #59

@bastelfreak
Copy link
Member Author

@dhxgit @killermoehre could I get your opinions here?

@bastelfreak
Copy link
Member Author

A note about the vim settings/indentation: these are the current settings used by upstream. We probably fuck up compatibility if we change them.

## Escaping
We don't want dirty escaping for variables in `echo`, we should prefer printf in these cases, here is a bad example:
```bash
echo -e "SUBSYSTEM==\"net\", ACTION==\"add\", DRIVERS==\"?*\", ATTR{address}==\"$2\", ATTR{dev_id}==\"0x0\", ATTR{type}==\"1\", KERNEL==\"eth*\", NAME=\"$1\"" >> $UDEVFILE
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is unfair to compare a double-quoted string containing double-quotes with a single-quoted string containing double-quotes.

Here is a better comparison:

echo 'SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="'"$2"'", ATTR{dev_id}=="0x0", ATTR{type}=="1", KERNEL=="eth*", NAME="'"$1"'"' >> $UDEVFILE
printf 'SUBSYSTEM=="net", ACTION=="add", DRIVERS=="?*", ATTR{address}=="%s", ATTR{dev_id}=="0x0", ATTR{type}=="1", KERNEL=="eth*", NAME="%s"\n' "$2" "$1" >> $UDEVFILE

In this case I would prefer the echo.

@killermoehre
Copy link
Contributor

The difference in the for-loops is a consequence of the different types you iterating over. You have either a list of strings or on the other hand real integers. Using seq to create a list of strings of integers is somehow wicked.

@bastelfreak
Copy link
Member Author

👍

discussed on IRC, c loops are prefered for intergers. this is a working draft and should be extended from time to time.

bastelfreak added a commit that referenced this pull request Feb 15, 2016
@bastelfreak bastelfreak merged commit 4858cdc into master Feb 15, 2016
@bastelfreak bastelfreak deleted the feat-styleguide branch February 15, 2016 23:13
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 this pull request may close these issues.

3 participants