-
Notifications
You must be signed in to change notification settings - Fork 123
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
cyberark/migrate-slosilo-gem #2853
base: master
Are you sure you want to change the base?
Conversation
# The recommended way to use this option is to pass a proc-ish that identifies the record. | ||
# Note the proc-ish can be a simple method name; for example in case of a Sequel::Model: | ||
# attr_encrypted :secret, aad: :pk | ||
def attr_encrypted *a |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method attr_encrypted
has a Cognitive Complexity of 7 (exceeds 5 allowed). Consider refactoring.
# unrecognized claims. | ||
# | ||
# @param token [JWT] pre-parsed token to verify | ||
def validate_jwt token |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method validate_jwt
has a Cognitive Complexity of 13 (exceeds 5 allowed). Consider refactoring.
require 'slosilo/errors' | ||
|
||
module Slosilo | ||
class Key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Class Key
has 24 methods (exceeds 20 allowed). Consider refactoring.
end | ||
|
||
# Try to convert by detecting token representation and parsing | ||
def self.JWT raw |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Method JWT
has a Cognitive Complexity of 6 (exceeds 5 allowed). Consider refactoring.
# untag, will use cache next time if available but no junk will be left | ||
docker rmi $iid | ||
|
||
rm $cidfile |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double quote to prevent globbing and word splitting.
f51b6ce
to
464377e
Compare
docker rm $cid | ||
|
||
# untag, will use cache next time if available but no junk will be left | ||
docker rmi $iid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double quote to prevent globbing and word splitting.
docker cp $cid:/app/spec/reports spec/ | ||
docker cp $cid:/app/coverage spec | ||
|
||
docker rm $cid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double quote to prevent globbing and word splitting.
cid=$(cat $cidfile) | ||
|
||
docker cp $cid:/app/spec/reports spec/ | ||
docker cp $cid:/app/coverage spec |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double quote to prevent globbing and word splitting.
|
||
cid=$(cat $cidfile) | ||
|
||
docker cp $cid:/app/spec/reports spec/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double quote to prevent globbing and word splitting.
cidfile=$(mktemp -u) | ||
docker run --cidfile $cidfile -v /app/spec/reports $iid bundle exec rake jenkins || : | ||
|
||
cid=$(cat $cidfile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Double quote to prevent globbing and word splitting.
Code Climate has analyzed commit 464377e and detected 308 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 82.6% (50% is the threshold). This pull request will bring the total coverage in the repository to 86.9% (-1.3% change). View more on Code Climate. |
464377e
to
d5a4ecf
Compare
Desired Outcome
Please describe the desired outcome for this PR. Said another way, what was
the original request that resulted in these code changes? Feel free to copy
this information from the connected issue.
Implemented Changes
Describe how the desired outcome above has been achieved with this PR. In
particular, consider:
Connected Issue/Story
Resolves #[relevant GitHub issue(s), e.g. 76]
CyberArk internal issue ID: [insert issue ID]
Definition of Done
At least 1 todo must be completed in the sections below for the PR to be
merged.
Changelog
CHANGELOG update
Test coverage
changes, or
Documentation
README
s) were updated in this PRBehavior
Security