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.

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

741 fully move to kotlin #742

Merged
merged 107 commits into from Nov 23, 2023
Merged

741 fully move to kotlin #742

merged 107 commits into from Nov 23, 2023

Conversation

Bdegraaf1234
Copy link
Collaborator

@Bdegraaf1234 Bdegraaf1234 commented Oct 25, 2023

Description: Move classes to kotline before migrating to ory to prevent having really obfuscated pull requests for significant changes in the implementation

Fixes #741

Checklist:

  • The Main workflow has succeeded
  • The Gatling tests have passed
  • I have logged into the portal running locally with default admin credentials
  • I have updated the README files if this change requires documentation update
  • I have commented my code, particularly in hard-to-understand areas

@Bdegraaf1234 Bdegraaf1234 linked an issue Oct 25, 2023 that may be closed by this pull request
@Bdegraaf1234 Bdegraaf1234 self-assigned this Oct 25, 2023
…ctually annotate the fields/parameters rather than the types (?)
…urceIntTest.kt and SourceDataResourceIntTest.kt
project: organization was not provided as valid json
Sourcetype: catalogversion was incorrectly set during save
subject: incomplete projectDTO was provided (no name). this could be resolved by looking up the name if an id is provided.
…e, and overwrites the field on get() to prevent inconsistencies. Only if there is no organization.name, the original field can be used.
Copy link
Member

@yatharthranjan yatharthranjan left a comment

Choose a reason for hiding this comment

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

LGTM. I have just quickly done a pass since I have not worked extensively on MP before and most of the files are renamed so hard to review the diff. I am approving given that this is sufficiently tested, the CI is passing and we will test it further in our stage environment.

Thanks @Bdegraaf1234 for this substantial piece of work and it will definitely make it easier to maintain.

Copy link
Collaborator

@peyman-mohtashami peyman-mohtashami left a comment

Choose a reason for hiding this comment

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

LGTM

@Bdegraaf1234 Bdegraaf1234 merged commit 9fe53af into dev Nov 23, 2023
3 of 5 checks passed
@Bdegraaf1234 Bdegraaf1234 deleted the 741-fully-move-to-kotlin branch November 23, 2023 12:59
@Bdegraaf1234 Bdegraaf1234 mentioned this pull request Mar 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"fully" move to kotlin
4 participants