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

feat: add 'mac' primitive for 'net.HardwareAddr' (MAC) #1165

Merged
merged 6 commits into from
Feb 6, 2024

Conversation

lukasmalkmus
Copy link
Contributor

@lukasmalkmus lukasmalkmus commented Jan 20, 2024

Very simple PR that adds a new string primitive mac to be interpreted as a net.HardwareAddr, just like ip -> netip.Addr.

Marking as draft as I haven't added any integration test for the generator. I wonder where that should go. Should I create a new spec in the _testdata folder or add to an existing one? Also not suer about the Array functions. Those exist for the UUID primitive but not for e.g. the IP one? Why is that?

Anything I need to keep in mind?

If I can get this landed, I'll send a PR to the docs repo, as well.

Cheers!

@ernado ernado marked this pull request as ready for review January 21, 2024 08:12
@ernado ernado marked this pull request as draft January 21, 2024 08:12
Copy link

codecov bot commented Jan 21, 2024

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (d5fceb0) 72.47% compared to head (9c84f12) 73.79%.
Report is 2 commits behind head on main.

Files Patch % Lines
conv/decode.go 0.00% 4 Missing ⚠️
conv/encode.go 0.00% 3 Missing ⚠️
gen/ir/faker.go 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1165      +/-   ##
==========================================
+ Coverage   72.47%   73.79%   +1.32%     
==========================================
  Files         187      188       +1     
  Lines       15049    12659    -2390     
==========================================
- Hits        10907     9342    -1565     
+ Misses       3608     2784     -824     
+ Partials      534      533       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lukasmalkmus lukasmalkmus force-pushed the lukasmalkmus/mac-primitive branch from d8e6968 to 094db93 Compare January 22, 2024 22:18
@lukasmalkmus lukasmalkmus marked this pull request as ready for review January 22, 2024 22:18
@lukasmalkmus
Copy link
Contributor Author

Forced pushed with some more changes I discovered by just global searching for UUID and going from there. Codecov isn't to happy but other types are not tested on those paths, as well.

Guess I need to alter one of the spaces with the mac format?

@ernado ernado requested a review from tdakkota January 23, 2024 11:51
Copy link
Member

@tdakkota tdakkota left a comment

Choose a reason for hiding this comment

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

Format should be handled here too:

switch f := s.Format; f {

@lukasmalkmus lukasmalkmus force-pushed the lukasmalkmus/mac-primitive branch from 67ea6eb to ad93fd3 Compare January 24, 2024 19:28
@lukasmalkmus lukasmalkmus force-pushed the lukasmalkmus/mac-primitive branch from ad93fd3 to d87b678 Compare February 6, 2024 00:45
@lukasmalkmus lukasmalkmus requested a review from tdakkota February 6, 2024 00:45
@tdakkota tdakkota enabled auto-merge February 6, 2024 04:44
@tdakkota tdakkota merged commit 3ae5daa into ogen-go:main Feb 6, 2024
14 checks passed
@lukasmalkmus lukasmalkmus deleted the lukasmalkmus/mac-primitive branch February 6, 2024 12:09
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