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

Add JavaDoc to HttpSessionEventPublisher#sessionIdChanged #16211

Closed
12OneTwo12 opened this issue Dec 4, 2024 · 2 comments · Fixed by #16216
Closed

Add JavaDoc to HttpSessionEventPublisher#sessionIdChanged #16211

12OneTwo12 opened this issue Dec 4, 2024 · 2 comments · Fixed by #16216
Assignees
Labels
status: ideal-for-contribution An issue that we actively are looking for someone to help us with type: enhancement A general enhancement

Comments

@12OneTwo12
Copy link
Contributor

12OneTwo12 commented Dec 4, 2024

I noticed that the sessionIdChanged method in the HttpSessionEventPublisher class does not have a Javadoc comment. In contrast, other methods in the same class (sessionCreated and sessionDestroyed) are well-documented with Javadoc.

The sessionIdChanged method appears to play an important role in handling session ID changes by publishing a HttpSessionIdChangedEvent. However, without proper Javadoc, it is harder to understand its purpose and behavior compared to the other methods in the class.

Is there a specific reason why this method lacks Javadoc?

Thank you for your time, and I look forward to your feedback! 😊

@12OneTwo12 12OneTwo12 added status: waiting-for-triage An issue we've not yet triaged type: enhancement A general enhancement labels Dec 4, 2024
@12OneTwo12 12OneTwo12 changed the title Why does the sessionIdChanged method in the HttpSessionEventPublisher class lack documentation? Why does the sessionIdChanged method in the HttpSessionEventPublisher class lack Javadoc? Dec 4, 2024
@jzheaux
Copy link
Contributor

jzheaux commented Dec 4, 2024

It looks like there is JavaDoc in the parent interface for that method, which may be why it wasn't added.

However, I think it would be fine to add the following to HttpSessionEventPublisher#sessionIdChanged:

/**
  * @inheritDoc 
  */

Are you able to submit a PR that adds this to sessionIdChanged?

@jzheaux jzheaux added status: ideal-for-contribution An issue that we actively are looking for someone to help us with and removed status: waiting-for-triage An issue we've not yet triaged labels Dec 4, 2024
@jzheaux jzheaux self-assigned this Dec 4, 2024
@jzheaux jzheaux changed the title Why does the sessionIdChanged method in the HttpSessionEventPublisher class lack Javadoc? Add JavaDoc to HttpSessionEventPublisher#sessionIdChanged Dec 4, 2024
@12OneTwo12
Copy link
Contributor Author

Thank you for the clarification!

That makes sense

I'll go ahead and prepare a PR to add the suggested Javadoc to HttpSessionEventPublisher#sessionIdChanged. Please let me know if there are any additional details you'd like me to consider

Thanks again for your guidance!

12OneTwo12 added a commit to 12OneTwo12/spring-security that referenced this issue Dec 5, 2024
jzheaux pushed a commit that referenced this issue Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: ideal-for-contribution An issue that we actively are looking for someone to help us with type: enhancement A general enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants