Skip to content
This repository has been archived by the owner on Oct 15, 2024. It is now read-only.

bashisms-test #1361

Merged
merged 1 commit into from
Feb 20, 2017
Merged

bashisms-test #1361

merged 1 commit into from
Feb 20, 2017

Conversation

e1528532
Copy link
Contributor

Purpose

Add bashisms regression check test as requested in #160

Checklist

  • commit messages are fine (with references to issues)
  • I ran all tests and everything went fine
  • I added unit tests
  • affected documentation is fixed
  • I added code comments, logging, and assertions
  • meta data is updated (e.g. README.md of plugins)

@markus2330 please review my pull request. checkbashisms will return 0 on success and 2 on a skipped file (e.g. it skips files which have a shebang pointing to awk). Therefore i handle 2 also as successful, as there is no real way to determine if skipping the file is ok (as it usually is) or not (e.g. cannot read file for whatsoever reason).
We might have to install the tool on the build server, otherwise it will skip the check.

@e1528532 e1528532 changed the title bashisms-test: add the test #160, fix forgotten files bashisms-test Feb 20, 2017
@markus2330
Copy link
Contributor

Thank you, great job! Please add label "ready to merge" if ready to merge.

@e1528532
Copy link
Contributor Author

Well the PR itself should be fine i think, we should just not forget to ensure that the checkbashisms is installed on the build server so it actually executes the test.

@markus2330 markus2330 merged commit e945477 into ElektraInitiative:master Feb 20, 2017
@markus2330
Copy link
Contributor

Thank you, devscripts should be installed at some of the build agents (e.g. debian-stable-mm). If it should be installed at other agents, too, please say which in #160.

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

Successfully merging this pull request may close these issues.

2 participants