Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat: upgrade BPDM to R24.12 #195

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

feat: upgrade BPDM to R24.12 #195

wants to merge 4 commits into from

Conversation

CDiezRodriguez
Copy link
Contributor

@CDiezRodriguez CDiezRodriguez commented Jan 16, 2025 •
edited
Loading

Description

Upgrade BPDM to R24.12 (5.2.0).

Close #189

Pre-review checks

Please ensure to do as many of the following checks as possible, before asking for committer review:

@CDiezRodriguez CDiezRodriguez self-assigned this Jan 16, 2025
@CDiezRodriguez CDiezRodriguez changed the title feat: upgrade bpdm to R24.12 feat: upgrade BPDM to R24.12 Jan 16, 2025
mgarciaLKS
mgarciaLKS approved these changes Jan 17, 2025
View reviewed changes
Copy link
Contributor

@mgarciaLKS mgarciaLKS left a comment

Choose a reason for hiding this comment

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

Tested and worked correctly

evegufy
evegufy requested changes Jan 21, 2025
View reviewed changes
Copy link
Contributor

@evegufy evegufy left a comment

Choose a reason for hiding this comment

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

LGTM! just one comment.

@nicoprow could you please have a look at this pr?

charts/umbrella/Chart.yaml Outdated Show resolved Hide resolved
@CDiezRodriguez @evegufy
Update charts/umbrella/Chart.yaml
5b43daf 
Co-authored-by: Evelyn Gurschler <[email protected]>
@CDiezRodriguez
Copy link
Contributor Author

CDiezRodriguez commented Jan 21, 2025 •
edited
Loading

@evegufy Can you run the checks again? I tested locally the parts that failed, and everything seems correct. The BPDM shouldn't affect values-test-data-exchange-iatp-mock test.

@CDiezRodriguez
Copy link
Contributor Author

CDiezRodriguez commented Jan 28, 2025 •
edited
Loading

It works fine now.

CDiezRodriguez
CDiezRodriguez commented Jan 29, 2025
View reviewed changes
charts/umbrella/values.yaml Outdated Show resolved Hide resolved
charts/umbrella/values.yaml Outdated Show resolved Hide resolved
nicoprow
nicoprow requested changes Feb 6, 2025
View reviewed changes
Copy link
Contributor

@nicoprow nicoprow left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Works fine as far as I can tell.

I adressed minor points. Especially, I would like to see the outdated comment to be removed for clarity.

When the comment is removed I'm alright to approve

charts/umbrella/values.yaml Outdated Show resolved Hide resolved
charts/umbrella/values.yaml Outdated Show resolved Hide resolved
@CDiezRodriguez
Copy link
Contributor Author

Waiting for the fix in PR #210 to be merged so it can function properly and work fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@evegufy evegufy evegufy requested changes

@nicoprow nicoprow nicoprow requested changes

@mgarciaLKS mgarciaLKS mgarciaLKS approved these changes

Assignees

@CDiezRodriguez CDiezRodriguez

Labels
None yet
Projects
Umbrella Helm Chart
Status: In Review
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Upgrade BPDM to R24.12
4 participants
@CDiezRodriguez @evegufy @nicoprow @mgarciaLKS