Skip to content

Issue 340 permission fix #345

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

Merged
merged 5 commits into from
Apr 18, 2017
Merged

Issue 340 permission fix #345

merged 5 commits into from
Apr 18, 2017

Conversation

zbeekman
Copy link
Collaborator

@zbeekman zbeekman commented Mar 21, 2017

Avg response time coverage on master
Issue Stats Codecov branch

This pull request (PR) is a:

  • Bug fix
  • Feature addition
  • Other, Please describe:

I certify that:

  • I have reviewed the contributing guidelines
  • If this PR is a work in progress I have added WIP: to the
    beginning of the PR title
  • If this PR is problematic for any reason, I have added
    DO NOT MERGE: to the beginning of the title
  • The branch name and title of this PR contains the text
    issue-<#> where <#> is replaced by the issue that this PR
    is addressing
  • I have deleted trailing white space on any lines that this PR
    touches
  • I have used spaces for indentation on any lines that this PR
    touches
  • I have included some comments to explain non-obvious code
    changes
  • I have run the tests localy (ctest) and all tests pass
  • Each commit is a logically atomic, self-consistent, cohesive
    set of changes
  • The commit message should follow these guidelines:
    • First line is directive phrase, starting with a capitalized
      imperative verb, and is no longer than 50 characters
      summarizing your commit
    • Next line, if necessary is blank
    • Following lines are all wrapped at 72 characters and can
      include additional paragraphs, bulleted lists, etc.
    • Use Github keywords where appropriate, to indicate the
      commit resolves an open issue.
  • I have signed Contributor License Agreement (CLA) by
    clicking the "details" link to the right of the licence/cla
    check and following the directions on the CLA assistant webpage
  • I have ensured that the test coverage hasn't gone down and added new unit tests to cover an new code added to the library

Summary of changes

Made installed scripts 555 permissions

Rationale for changes

Installed software should not be group or world writeable, since this is a huge security vulnerability, obviously.

For contributors and SI team members with code review priviledges

@rouson
Copy link
Member

rouson commented Mar 21, 2017

Let's move Summary of Changes and Rationale for Changes to the top of this file and the template(s).

@codecov
Copy link

codecov bot commented Mar 21, 2017

Codecov Report

Merging #345 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #345   +/-   ##
=======================================
  Coverage   46.22%   46.22%           
=======================================
  Files           3        3           
  Lines        1045     1045           
  Branches      201      201           
=======================================
  Hits          483      483           
  Misses        483      483           
  Partials       79       79

@zbeekman zbeekman force-pushed the issue-340-permission-fix branch from 608f28e to 2e38f3c Compare April 18, 2017 20:43
 - Update `caf` and `cafrun` to use the more portable shebang
   `#!/usr/bin/env bash` rather than the old `#!/usr/bin/bash`
 - The user/system bash will get used
@zbeekman
Copy link
Collaborator Author

@rouson if you have a chance to look at this and give it a "LGTM" or provide comment that would be great. Feel free to merge too, I'm done. If this happens in the next 5 min I'll try to upload release assets before going to the gym. Otherwise I'll do it when I return.

@rouson
Copy link
Member

rouson commented Apr 18, 2017

LGTM

Approved with PullApprove

@rouson
Copy link
Member

rouson commented Apr 18, 2017

@zbeekman, I'll let you merge in case you want to wait for the tests to complete.

@zbeekman
Copy link
Collaborator Author

zbeekman commented Apr 18, 2017 via email

@rouson rouson merged commit 00490f1 into master Apr 18, 2017
@rouson rouson deleted the issue-340-permission-fix branch April 18, 2017 22:41
@zbeekman
Copy link
Collaborator Author

zbeekman commented Apr 19, 2017

LGTM

Approved with PullApprove Approved with PullApprove

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