Skip to content

Conversation

PedroSFreitas
Copy link
Member

I could have squashed the commits to avoid a big chunk of changes. Although I believe it's easier for the team to see all the changes and even mark a few for a deep review.

This first pull is 100% about refactoring, no new functionality is being added.
I'll work on that in the next weekend or so.

Make sure to read the full commit messages.

I'd love any critics, negative or positive.

This I'm not entirely sure. If calling  will, of course, exit the script then why have a  after it?
According to a few webpages, brace expansion in `sh` is not defined (POSIX).
Resources:
    http://mywiki.wooledge.org/Bashism
    https://github.com/koalaman/shellcheck/wiki/SC2039

Note: although I'm not sure if this will be a problem.
- `local` is undefined by POSIX (http://mywiki.wooledge.org/Bashism)
- declaration and assigment should be separated (https://github.com/koalaman/shellcheck/wiki/SC2155)
- and using $() rather than `code` (https://github.com/koalaman/shellcheck/wiki/SC2006)
- changing variable uses to a common use inside the script;
- adding TODO statement in unfinished functions;
- changing `expr` to built-in $(()) ;
- using `||` rather than `-o`;
@noptrix noptrix merged commit 5c5c56c into BlackArch:master Aug 10, 2017
@noptrix
Copy link
Contributor

noptrix commented Aug 10, 2017

thanks a lot for this!

@PedroSFreitas
Copy link
Member Author

> noptrix commented 8 minutes ago

thanks a lot for this!

This is nothing, mate. Just some basic refactoring. No need for a thanks.
I'll try to implement a few things, I saw that some functions aren't working as expected. But this is such a nice application that I believe we should give it some focus. :)

I'll just ask your patient with English because it's not my native language, I'm from Brazil, so I may write some commit messages wrong.

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.

2 participants