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

Update @SourceStreams and @SinkStreams docs to clarify that they are required #46

Merged
merged 5 commits into from
Jan 9, 2024

Conversation

nicoweidner
Copy link
Contributor

Resolves #45

@gunnarmorling
Copy link
Contributor

LGTM. One additional thing we could do is raising a warning in the annotation processor which creates the META-INF file if there are no streams at all. While it's technically still valid (if a user doesn't want to connect to any Decodable streams at all), I think it should be the exceptional case. So more users would benefit from that build-time warning than would be irritated by a "false" warning. WDYT?

@nicoweidner
Copy link
Contributor Author

LGTM. One additional thing we could do is raising a warning in the annotation processor which creates the META-INF file if there are no streams at all. While it's technically still valid (if a user doesn't want to connect to any Decodable streams at all), I think it should be the exceptional case. So more users would benefit from that build-time warning than would be irritated by a "false" warning. WDYT?

Ok, I tried to change the processor so it always runs and prints a warning when neither sources nor sinks are specified. It now processes all annotations so that it's not skipped entirely if the annotations are not present - but in turn, it doesn't mark the annotations as processed so other processors can process their annotations afterwards.

I also fixed a NPE I noticed by chance.

@nicoweidner
Copy link
Contributor Author

@gunnarmorling quick ping

Copy link
Contributor

@gunnarmorling gunnarmorling left a comment

Choose a reason for hiding this comment

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

LGTM, thx!

@nicoweidner nicoweidner merged commit bd1d1df into decodableco:main Jan 9, 2024
1 check passed
@nicoweidner nicoweidner deleted the issue-45-stream-docs branch January 9, 2024 17:01
@github-actions github-actions bot added the released Issue has been released label Jan 17, 2024
Copy link

🎉 This issue has been resolved in v1.0.0.Beta3 (Release Notes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released Issue has been released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update docs for source and sink streams to emphasize that the annotations are required
2 participants