Skip to content
This repository has been archived by the owner on Jan 20, 2025. It is now read-only.

New Release 1.1.0 #9

Merged
merged 11 commits into from
Feb 24, 2023
Merged

New Release 1.1.0 #9

merged 11 commits into from
Feb 24, 2023

Conversation

fabiodmota
Copy link
Contributor

Fixed css style guidelines
Fixed whitespaces on helm charts

@fabiodmota fabiodmota changed the title New Release 1.0.2 New Release 1.0.1 Feb 21, 2023
@SebastianBezold
Copy link
Contributor

Hi @fabiodmota ,
please ask someone to do a content review for all the CSS and React component changes, since i cannot judge, if that is what you are aiming for.
The Chart changes seem fine. So i can approve that part.
Beside that. Just upgrading the version on a bugfix level seems to be quite small, considering you added completely new React components. But again: I cannot tell, if this is only a bugfix or already a new feature.
So please, who ever can do this review for the React parts, point out, if the bugfix version upgrade is ok.

@@ -52,6 +52,15 @@ export const getPortalLink = () => {
return "https://portal.dev.demo.catena-x.net/";
};

export const getLogoutLink = () => {
const hostname = getHostname();
if (hostname === "country-risk-dashboard.dev.demo.catena-x.net")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can't be the right way on how to handle this.

This should be (in my opinion) be configurable from the helm chart itself and not hard coded.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabiodmota
as discussed in the teams chat, please refer to those as illustrative commits for the approach we used:
catenax-ng/tx-portal-frontend@1a990af
catenax-ng/tx-portal-frontend@b1d3301
Even tough implementation-wise it's not entirely the same, I was inspired with this approach by the traceability team.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fabiodmota
Just to be clear, additionally to the commits above, you also need to enable the env vars in the helm chart.
Reference PR: catenax-ng/tx-portal-cd#5

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Corrected @SebastianBezold ,

Since the new approach i upgraded the version to 1.1.0

As I said the css would have already been approved and validated by the @evegufy/julia team, now it is also corrected for the hardcoded envs thanks for that @evegufy

Also added the arq42 documentation approved by Alexander.AK.Keppler@bmw.de

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fabiodmota , I cannot find the docs review here. Can you please point me to it, or ask Alexander to document it here in this PR?
@evegufy can you confirm, that the styleguide requirements are now met? cannot find that approval here either.
Please keep these alignments/approvals transparent. For me and anyone else not being part in your private conversations, it is quite hard to follow

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @SebastianBezold, I can confirm the arc42 documentation is up to date.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @fabiodmota , I cannot find the docs review here. Can you please point me to it, or ask Alexander to document it here in this PR? @evegufy can you confirm, that the styleguide requirements are now met? cannot find that approval here either. Please keep these alignments/approvals transparent. For me and anyone else not being part in your private conversations, it is quite hard to follow

@SebastianBezold There is currently a review by Julia going on as part of this ticket: https://jira.catena-x.net/browse/BPDM2-469

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @SebastianBezold , regarding the styleguides requirements they have been approved as you can see in the last comment in this ticket by Julia (https://jira.catena-x.net/browse/BPDM2-469). Thanks

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jjeroch: Could you please give your styleguide approval for QG5 here as requested by @Siegfriedk during the QG5 review.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @evegufy @alexsilva-CGI and @alexKeppler ,
thank you for the input, but please refrain from doing reviews or posting updates in tools, that are not open for the open source community. This makes this whole effort intransparent.

So for this last time, i'll work my way through the comments. For upcoming reviews, please do them transparently for everyone

@fabiodmota fabiodmota changed the title New Release 1.0.1 New Release 1.1.0 Feb 22, 2023
@SebastianBezold SebastianBezold merged commit ae79385 into eclipse-tractusx:main Feb 24, 2023
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants