Explicit Compliance #1495
Merged
Explicit Compliance #1495
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov Report
@@ Coverage Diff @@
## master #1495 +/- ##
==========================================
- Coverage 37.21% 36.33% -0.89%
==========================================
Files 1002 1030 +28
Lines 22433 23734 +1301
Branches 6180 6554 +374
==========================================
+ Hits 8348 8623 +275
- Misses 12766 13732 +966
- Partials 1319 1379 +60
Continue to review full report at Codecov.
|
sarahd93
force-pushed
the
explicit_compliance_gsa8
branch
4 times, most recently
from
7f7ccf6
to
620e657
Compare
bjoernricks
suggested changes
Jul 29, 2019
sarahd93
force-pushed
the
explicit_compliance_gsa8
branch
2 times, most recently
from
aee582a
to
dc99c54
Compare
bjoernricks
reviewed
Jul 31, 2019
There was a problem hiding this comment.
The PR looks good besides copy and paste mistakes.
Two major things must be done to get this PR into shape:
- Always use camel case in GSA. For new and changed code we should only use camelCase for variable names. The command classes need to convert the camelCase to underscore names for gsad.
- Avoid duplicate code. Just copy code where it needs to be copied. Otherwise try to refactor old code to be more flexible e.g. by allowing to change titles.
| @@ -57,7 +57,10 @@ const withEntitiesContainer = ( | |||
| const mapStateToProps = (state, {gmp}) => { | |||
| const eSelector = entitiesSelector(state); | |||
| const pSelector = getPage(state); | |||
| const filter = pSelector.getFilter(gmpname); | |||
| let filter = pSelector.getFilter(gmpname); | |||
There was a problem hiding this comment.
Would have been nice to create a distinct PR for such a feature next time.
| comment, | ||
| hostsOrdering, | ||
| id, | ||
| in_assets, |
There was a problem hiding this comment.
All new variables should be camelCase
Suggested change
| in_assets, | |
| inAssets, |
| } | ||
|
|
||
| EditDialogComponent.propTypes = { | ||
| family_name: PropTypes.string, |
There was a problem hiding this comment.
Suggested change
| family_name: PropTypes.string, | |
| familyName: PropTypes.string, |
| const data = { | ||
| policy, | ||
| policyName, | ||
| family_name, |
There was a problem hiding this comment.
Suggested change
| family_name, | |
| familyName, |
| const { | ||
| policy, | ||
| policyName, | ||
| family_name, |
There was a problem hiding this comment.
Suggested change
| family_name, | |
| familyName, |
| return [...nvts].sort(compare); | ||
| }; | ||
|
|
||
| class EditDialogComponent extends React.Component { |
There was a problem hiding this comment.
is the code in this file different from scan configs?
sarahd93
force-pushed
the
explicit_compliance_gsa8
branch
from
0a98c8e
to
4787a67
Compare
bjoernricks
approved these changes
Jul 31, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Checklist: