-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
Next version – various refactors and cleanups #6
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This improves parser performance a little and cleans up the code.
SSE is a text-based protocol, which uses the UTF-8 encoding. Therefore it makes no sense to use raw byte slices in the API and the implementation, reasons being: - Giving a false impression to users that raw data can be safely sent using this protocol - Necessity of unsafe string to byte slice conversions of user input for performance – immutability in userland is impossible to enforce - Necessity of specifying ownership of byte slices in comments – with safely created strings, one must not worry about copying it, to prevent it being mutated at source - More memory footprint of sse.Message - More code The following changes were made: - Remove ability to add raw bytes as data to server events; AppendText is removed, in favour of AppendData taking in strings - Change the client event's data type to string - Remove fmt.Stringer impl of client event (unnecessary and subjective) - Many internal usages of []byte have been replaced with string
Also make cloning of chunks lazy.
This refactors chunk handling to not include newlines aswell.
This also modifies the FieldParser to output comments.
- Extract single-line string logic into separate type, messageField - Rename EventID constructors: NewEventID -> NewID, MustEventID -> ID - Create EventName type with constructors named similarly to EventID's constructors - Use these new types and constructors in Message and other code
To adhere to the spec nomenclature.
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #6 +/- ##
==========================================
- Coverage 96.27% 94.91% -1.37%
==========================================
Files 13 13
Lines 1020 963 -57
==========================================
- Hits 982 914 -68
- Misses 30 37 +7
- Partials 8 12 +4
☔ View full report in Codecov by Sentry. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See changelog and commits.