-
Notifications
You must be signed in to change notification settings - Fork 99
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
fix(report): corrections to report #526
Conversation
please correct commit message as per conventional changelog https://github.com/eclipse/sw360/wiki/Dev-DoD-and-Style |
maybe something not checked in? - from the travis CI log:
|
@thegrumpylion ehem, could you please have a look at the a) wrong commit style and b) test issue |
@thegrumpylion I thing despite maybe not running tests, there are more fundamental probems with the contribution. Tried this two times now, but branch is even not compiling on my (vagrant) machine....
thrift and java version outputs:
|
@@ -194,9 +197,9 @@ private void fillOwnerGroup(XWPFDocument document, Project project) throws XmlEx | |||
} | |||
|
|||
private void fillAttendeesTable(XWPFDocument document, Project project) throws XmlException, TException { | |||
XWPFTable table = document.getTables().get(0); | |||
XWPFTable table = document.getTables().get(OVERVIEW_TABLE_INDEX); |
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.
seems also to me that OVERVIEW_TABLE_INDEX
is at least not declared in the same class, so javac telling me it is missing this sounds likely.
910d984
to
0db2c3e
Compare
0db2c3e
to
02dba3a
Compare
...-licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/outputGenerators/DocxGenerator.java
Outdated
Show resolved
Hide resolved
...-licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/outputGenerators/DocxGenerator.java
Show resolved
Hide resolved
...-licenseinfo/src/main/java/org/eclipse/sw360/licenseinfo/outputGenerators/DocxGenerator.java
Show resolved
Hide resolved
@@ -206,6 +206,7 @@ struct Release { | |||
53: optional set<string> operatingSystems, | |||
54: optional COTSDetails cotsDetails, | |||
55: optional EccInformation eccInformation, | |||
56: optional set<string> softwarePlatforms, |
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 new field is completely independent from the list of software platforms at the component level? Why is it needed?
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 don't understand the first question. It is needed because we report on the release level and not on the component level.
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.
There is already a field "softwarePlatforms" at the component level. As a release always belongs to a component I was wondering if the fields should really be independent. Or if there should be some logic like "the list of software platforms of a component is the aggregation of software platform lists of its releases" now that we have this field at release level.
Beside that your core idea here seems to be that the content of software platforms can change between different releases of the same component. Is that correct? Otherwise you could just get the information from the component of the release even when you report on release level.
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.
There is already a field "softwarePlatforms" at the component level. As a release always belongs to a component I was wondering if the fields should really be independent. Or if there should be some logic like "the list of software platforms of a component is the aggregation of software platform lists of its releases" now that we have this field at release level.
Since a release already has it's own "operating systems" & "programming languages" i assumed that "software platforms" should follow suit. If this is wrong, what is the purpose of "operating systems" & "programming languages" on the release level?
Beside that your core idea here seems to be that the content of software platforms can change between different releases of the same component. Is that correct? Otherwise you could just get the information from the component of the release even when you report on release level.
Yes, that is correct. Isn't the component suppose to be more of an abstract/template of a release and the each release is what actually goes into the project?
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.
maybe the field softwarePlatforms should not been created at the first hand, but if in the report, it should be taken from the component where the release belongs to. I think a similar case is type (OSS, COTS, Freeware, ...) where the field value is taken from the component when listing releases.
.../sw360-portlet/src/main/webapp/html/components/includes/releases/editReleaseInformation.jspf
Outdated
Show resolved
Hide resolved
3c674cb
to
49f9f8e
Compare
7116c18
to
04d1965
Compare
To fix the tests, you need to blacklist |
04d1965
to
3326038
Compare
As per: siemens#18 Signed-off-by: Nikolas Sepos <[email protected]>
3326038
to
2fc4bd3
Compare
Signed-off-by: Leo von Klenze <[email protected]>
@mcjaeger What about your comment about the software platforms at release level? |
not covered from issue: #588
more info here: #588 |
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.
approving it, because this PR solves some issues from #588 but not all. Should be merged, because it is improvement.
As per: siemens#18
Signed-off-by: Nikolas Sepos [email protected]