Skip to content

[521] Add support for new API for adding groups for versions >= 4.3.4 - #522

Merged
alanking merged 1 commit into
DICE-UNC:masterfrom
JustinKyleJames:issue_521
Jul 29, 2025
Merged

[521] Add support for new API for adding groups for versions >= 4.3.4#522
alanking merged 1 commit into
DICE-UNC:masterfrom
JustinKyleJames:issue_521

Conversation

@JustinKyleJames

Copy link
Copy Markdown
Collaborator

I tested adding groups via metalnx in 4.3.3, 4.3.4, and 4.3.5. The jargon-core tests also passed.

@korydraughn korydraughn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Change look good.

Please add a test for this. We need to make sure the new test passes against 4.3.3 and 5.0.1.

Comment thread jargon-core/src/main/java/org/irods/jargon/core/pub/UserGroupAOImpl.java Outdated
Comment thread jargon-core/src/main/java/org/irods/jargon/core/packinstr/GeneralAdminInp.java Outdated

@korydraughn korydraughn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good. Squash it.

@JustinKyleJames

Copy link
Copy Markdown
Collaborator Author

Change look good.

Please add a test for this. We need to make sure the new test passes against 4.3.3 and 5.0.1.

There is already a test that covers this. It is UserGroupAOImplTest.testAddUserGroup. It covers this.

As for testing against 4.3.3 and 5.0.1, that was tested via point Metalnx to iRODS servers at these versions (as well as 4.3.4).

We don't have a testing environment setup to run the unit test listed above in 4.3.3 and 5.0.1 at the moment though I did open an issue to create a 5-0 testing environment setup.

@JustinKyleJames

Copy link
Copy Markdown
Collaborator Author

Looks good. Squash it.

Done

@korydraughn korydraughn left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Pound it.

@JustinKyleJames

Copy link
Copy Markdown
Collaborator Author

Pound it.

done

@alanking
alanking merged commit 9f9c743 into DICE-UNC:master Jul 29, 2025
1 of 2 checks passed
@JustinKyleJames
JustinKyleJames deleted the issue_521 branch August 1, 2025 16:05
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.

3 participants