Skip to content

Refactor run.sh script in a backward-compatible manner#9

Open
Phikimon wants to merge 2 commits intolisitsynSA:masterfrom
Phikimon:master
Open

Refactor run.sh script in a backward-compatible manner#9
Phikimon wants to merge 2 commits intolisitsynSA:masterfrom
Phikimon:master

Conversation

@Phikimon
Copy link

First of all, I would leave all the building/cleaning to Make,
because there is no need to reinvent building mechanics.
But since run.sh is already used for building/cleaning/testing, I
decided not to disrespect things as they are and just refactor
the code, allowing usage in the original way.

What are the enhancements?

  1. Extra caution is used to make sure we do not do some unintended stuff:
    current directory is compared against script source directory; EUID
    is also checked to make sure we are not root. Usage of -f option
    makes following these conditional optional.
  2. Verbose option is added, but for now it is pretty dumb and just prints
    full files' paths. I believe however this option will prove itself
    useful once some custom debug-printing lines are added.
  3. Code redundancy is minimized: main part of run.sh is put in the
    shared directory.
  4. Code is refactored, (hopefully) improving general readability. Since
    we do not use Make, let's at least have our own targets with
    blackjack and...

First of all, I would leave all the building/cleaning to Make,
because there is no need to reinvent building mechanics.
But since run.sh is already used for building/cleaning/testing, I
decided not to disrespect things as they are and just refactor
the code, allowing usage in the original way.

What are the enhancements?
1. Extra caution is used to make sure we do not do some unintended stuff:
   current directory is compared against script source directory; EUID
   is also checked to make sure we are not root. Usage of -f option
   makes following these conditional optional.
2. Verbose option is added, but for now it is pretty dumb and just prints
   full files' paths. I believe however this option will prove itself
   useful once some custom debug-printing lines are added.
3. Code redundancy is minimized: main part of run.sh is put in the
   shared directory.
4. Code is refactored, (hopefully) improving general readability. Since
   we do not use Make, let's at least have our own targets with
   blackjack and...
@Phikimon
Copy link
Author

Removed debug comment

@Phikimon Phikimon force-pushed the master branch 2 times, most recently from a0bedb3 to eeaef3c Compare October 21, 2020 13:19
@Phikimon
Copy link
Author

Accidentally pushed hometask onto the same branch, just fixed that.

@Phikimon
Copy link
Author

I just got to know that it is preferrable to point out exactly which (potential) bugs are being fixed:

  1. If old run.sh script is ran from some other directory, unwanted stuff may happen, like deleting "./build" directory unconditionally. Also running as root is dangerous. This makes my feel a bit anxious.
  2. No handling of iteration number argument -- most of the time it just silences error(try ./run.sh a), and sometimes it yields error (try ./run.sh "\$").
  3. If program being executed returns some error code, nobody cares and test iterations continue running... due to default-set "set +e". Actually, there is almost no error handling in the script.

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.

1 participant