Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

feat: Implement oneof tag #103

Merged
merged 18 commits into from
Jun 8, 2020
Merged

feat: Implement oneof tag #103

merged 18 commits into from
Jun 8, 2020

Conversation

andrew-werdna
Copy link
Contributor

  • This adds support for a faker:"oneof: val1 :val2 :val3"
  • Currently only string and int is suppported
  • I would be happy to add support for more types at a later time
  • This looks like it might fix (or at least partially fix) Bool values or OneOf #82

@codecov
Copy link

codecov bot commented Jun 7, 2020

Codecov Report

Merging #103 into master will increase coverage by 0.15%.
The diff coverage is 95.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #103      +/-   ##
==========================================
+ Coverage   94.79%   94.95%   +0.15%     
==========================================
  Files          10       10              
  Lines        1019     1050      +31     
==========================================
+ Hits          966      997      +31     
  Misses         29       29              
  Partials       24       24              
Impacted Files Coverage Δ
faker.go 91.01% <95.55%> (+0.59%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 492cb34...4c44fc2. Read the comment docs.

@bxcodec
Copy link
Owner

bxcodec commented Jun 7, 2020

Can you resolve the conflict, please?

I can't merge to your PR

@andrew-werdna
Copy link
Contributor Author

oh yes of course! my apologies, I didn't notice that.

@andrew-werdna
Copy link
Contributor Author

@bxcodec my apologies, I didn't notice the conflicts somehow. I just caught my branch up with your master, and fixed the merge conflicts!

@@ -113,6 +113,9 @@ const (
BoundaryEnd = "boundary_end"
Equals = "="
comma = ","
colon = ":"
Copy link
Owner

@bxcodec bxcodec Jun 7, 2020

Choose a reason for hiding this comment

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

Are there any reasons why we choose colon for separator?

From the issue, he expects some kind of foo,bar,x,y I'm okay with either, but it will be great if we also considering the user experience?

How usually people separate a list of items? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah I thought about that. I can switch it if you want. I basically was lazy and didn't want to do the work to make oneof: value1, value2, value3. I didn't have an actually good reason for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have to run an errand for a little bit right now. But when I get back in an hour or 2, I will be happy to make the user experience better.
i.e. support instead oneof: value1, value2, value <- use colon for the oneof tag only. Then use commas to actually separate the "arguments" to the tag.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bxcodec I think I have it in place now. It now uses , to separate the "arguments" to the oneof tag

Copy link
Owner

Choose a reason for hiding this comment

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

Nice, thanks bro @andrew-werdna

@andrew-werdna andrew-werdna changed the title Setup oneof tag Implement oneof tag Jun 8, 2020
Copy link
Owner

@bxcodec bxcodec left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the pr bro @andrew-werdna

Do you have a plan for other data types in the near of the future?
I'm planning to move this to v3.5.0 with the full support of OneOfTag

@bxcodec bxcodec changed the title Implement oneof tag feat: Implement oneof tag Jun 8, 2020
@andrew-werdna
Copy link
Contributor Author

I'm down for whenever/however I can help. I'm thinking I'll probably contribute the most on weekends. I'm happy to coordinate with you at your convenience. I can wait, or jump in about for float this weekend. Or look at other issues too in the coming weekends. Just let me know.

@bxcodec
Copy link
Owner

bxcodec commented Jun 8, 2020

Thanks bro, float will be great as well. But take your time, no rush, I'm in debt for your help 🙏 .

I'm merging this now

@bxcodec bxcodec merged commit 463a71e into bxcodec:master Jun 8, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants