-
Notifications
You must be signed in to change notification settings - Fork 2
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
Feature/176 - Compare and Search views #181
Conversation
- Changed native selects to DropdownSelect - Styled the view
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.
This was a pretty big pull request.
I don't know if those backend scripts are still needed, maybe @lakooi has some idea on it?
Left few suggestions how to fix the codacy issues and there where few functions that where repeated that can be fixed, but it's also ok if not. Those where mostly suggestions.
All in all looks pretty solid.
@@ -0,0 +1,161 @@ | |||
#!/bin/sh |
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.
I have no idea about this backend scripts if they are needed or not. Or are they really part of this pull request?
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.
Removed this file from the branch as it contained our backend credentials
frontend/Dockerfile
Outdated
EXPOSE 3000 |
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.
Same as above
}); | ||
//Done twice since 2 builds | ||
const secondBuildByBuildNumber = comparedDataState[1] | ||
.filter(({ test_cases }) => { |
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.
this function is repeated above so it could be moved to a single function
}); | ||
|
||
const first_not_matching = firstBuildByBuildNumber.filter(test_case => { | ||
return !matching_array.some(matcher => { |
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.
here is also some repetition.
@@ -0,0 +1,56 @@ | |||
import React from 'react'; | |||
import Status from './Status'; | |||
// import { dashify } from '../../utils/helpers'; |
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.
Do we need these comments anymore?
frontend/src/pages/Comparison.jsx
Outdated
let found = 0; | ||
let metadata_len = json.metadata.length; | ||
while (index < metadata_len && found === 0) { | ||
if (json.metadata[index].metadata_name === name) { |
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.
if (json.metadata[index].metadata_name === name) { | |
if (json.metadata[parseInt(index)].metadata_name === name) { |
This could fix the codacy issue
frontend/src/pages/Comparison.jsx
Outdated
}; | ||
json.metadata.push(temp); | ||
} else { | ||
json.metadata[index].metadata2_value = |
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.
json.metadata[index].metadata2_value = | |
json.metadata[parseInt(index)].metadata2_value = |
frontend/src/pages/Comparison.jsx
Outdated
} else { | ||
json.metadata[index].metadata2_value = | ||
meta.metadata_value; | ||
json.metadata[index].suite2_id = meta.suite_id; |
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.
json.metadata[index].suite2_id = meta.suite_id; | |
json.metadata[parseInt(index)].suite2_id = meta.suite_id; |
frontend/src/pages/Comparison.jsx
Outdated
json.metadata[index].metadata2_value = | ||
meta.metadata_value; | ||
json.metadata[index].suite2_id = meta.suite_id; | ||
json.metadata[index].test2_run_id = |
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.
json.metadata[index].test2_run_id = | |
json.metadata[parseInt(index)].test2_run_id = |
See issue #176
This PR's commits should be squashed into one when merging.