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

Remove finalize() #725

Draft
wants to merge 9 commits into
base: master
Choose a base branch
from
Draft

Remove finalize() #725

wants to merge 9 commits into from

Conversation

jbescos
Copy link
Member

@jbescos jbescos commented Mar 15, 2024

#720

I found only few places in mail-api. Some of them are abstract and they don't have usage in mail-api, so I will need to check angus-mail.

It is probably out of the scope of this issue, but I couldn't resist to remove Objects.requireNonNull(this);. Is there any reason to have that?.

Signed-off-by: Jorge Bescos Gascon <[email protected]>
Copy link
Contributor

@jmehrens jmehrens left a comment

Choose a reason for hiding this comment

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

@jbescos You can feel free to notify me or add me as a reviewer if you want me to look at something. I missed this one because I forgot to check for new PRs.

api/src/main/java/jakarta/mail/Folder.java Outdated Show resolved Hide resolved
@jmehrens
Copy link
Contributor

jmehrens commented Mar 18, 2024

Also more of housekeeping thing but, I didn't create a clone issue in angus mail. There are finializers there too that need to be removed. I'm not sure of the workflow but I noticed that it is possible to:

  1. Create an issue in JakartaMail.
  2. Link it to a PR in JakartaMail.
  3. Link the JakartaMail issue to another PR in AngusMail.
  4. Use the JakartaMail issue number and title in the changes.txt of JakartaMail.
  5. Use the AngusMail PR number but the JakartaMail issue title in the changes.txt of AngusMail.

The result is one less issue ticket being created without any numbering conflicts.

Signed-off-by: Jorge Bescos Gascon <[email protected]>
@jbescos jbescos marked this pull request as draft March 20, 2024 08:51
Signed-off-by: Jorge Bescos Gascon <[email protected]>
jbescos added 4 commits March 27, 2024 07:41
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Signed-off-by: Jorge Bescos Gascon <[email protected]>
@@ -32,6 +32,7 @@
import java.util.List;
import java.util.Vector;
import java.util.concurrent.Executor;
import java.util.concurrent.atomic.AtomicBoolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

Dead import?

jbescos added 2 commits April 8, 2024 14:45
Signed-off-by: Jorge Bescos Gascon <[email protected]>
Signed-off-by: Jorge Bescos Gascon <[email protected]>
@jmehrens
Copy link
Contributor

Before I forget, you'll have to write up some COMPAT.txt notes for close:

  1. No arg close no longer hostile if closed.
  2. No arg close is now required to be called to signal event thread to quit.
  3. Obviously users must manage resources as the finalizers are removed.

Understand that this is blocked by version changes and PR is still draft so there could be more.

@jmehrens jmehrens self-requested a review April 13, 2024 06:41
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.

2 participants