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

[ELY-2828] Replace wildcard imports with specific imports inside X500PrincipalBuilderTest.java #2220

Closed
wants to merge 4 commits into from

Conversation

Samith10
Copy link
Contributor

@Samith10 Samith10 commented Oct 4, 2024

@fjuma
Copy link
Contributor

fjuma commented Oct 4, 2024

@Samith10 Just a small comment, looks like your changes for ELY-2826 also ended up in this PR.

@rsearls
Copy link
Contributor

rsearls commented Oct 4, 2024

1+ looks good

@luck3y
Copy link
Contributor

luck3y commented Oct 4, 2024

@Samith10 FYI you seem to have changes from a previous commit included in this PR as well, do you need some help removing that one? (If you look at the changes there are two files changed).

@luck3y
Copy link
Contributor

luck3y commented Oct 4, 2024

You can use something like:

$ git rebase -i HEAD~2
use the instructions on the screen to drop the unwanted commit (change the letter to 'd') then save.

[Note corrected git command from 'git commit rebase' to 'git rebase'.

Copy link
Contributor

@PrarthonaPaul PrarthonaPaul left a comment

Choose a reason for hiding this comment

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

Hello @Samith10 Thank you!
I have left a small comment that can help resolve the error you see in the CI

@@ -18,7 +18,7 @@

package org.wildfly.security.x500;

import static org.junit.Assert.*;
import static org.junit.Assert.assertEqua;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is supposed to be assertEquals?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sorry just making tiny changes to figure out merge issues

@PrarthonaPaul
Copy link
Contributor

Hello @Samith10
It seems like you have 4 commits, 2 related to ELY-2828 and 2 related to 2826.
Could you please merge the ones related to ELY-2828 and drop the ones associated with ELY2826? Please feel free to let us know if you have any questions.

@Samith10 Samith10 closed this Oct 4, 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.

5 participants