-
Notifications
You must be signed in to change notification settings - Fork 48
Add Support for Threadsafe and NotThreadSafe annotations #188
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
Conversation
In a Java test modules you could literarily define an annotation with those names - it doesn't have to be the "real jar". Do you want to give that a short or should I try to streamline it a bit more? Idk the exact steps without looking into it, let me know if you'd need a hand here. Alternatively you could do this on one of the sample apps -- we CI verify them all, so declare the annotation there and see that some type indeed got the Sendable conformance by assigning it to some |
I'll look into it later this week and let you know if I can figure out the automated test way.
Oh didn't know that this was done in CI. If I can't get the first part right, I can do this part |
We actually rely on the samples a lot for correctness so yeah, that'd definitely be a viable way to verify things as well :) |
cfef570
to
fe47e58
Compare
@@ -0,0 +1,6 @@ | |||
package com.example.swift; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
missing license header?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks for including the snippet
I updated one of the Samples to check for ThreadSafe. This still has the hole where there isn't a way to test something is not Sendable (which NotThreadSafe does). I can look later in the week for other testing of not sendable depending on what you think |
That's okey, thanks :) |
Closes #129
This adds support for threadsafe, immutable, and notthreadsafe annotations. I was able to test this locally making jars, which I can attach if needed, but couldn't do any automated testing due to the fact that these types are not built in to Java, so I couldn't use the normal translator tests. I'm open to hearing if there are any good ways of doing automated testing for this (we could possibly build a jar in CI with the given dependencies and use that, but it's probably not great to be doing that during unit tests, especially since I think we will need to download dependencies if we want to use the real annotations in the ticket)