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

Implement Formatter u16, add u16 tests #13

Closed
wants to merge 1 commit into from
Closed

Implement Formatter u16, add u16 tests #13

wants to merge 1 commit into from

Conversation

ghost
Copy link

@ghost ghost commented Dec 7, 2017

No description provided.

@ghost
Copy link
Author

ghost commented Dec 7, 2017

@vitiral are there any plans to continue with this project? I would be interested in implementing some more types, namely i8, u8 and i16, probably some more too

@vitiral
Copy link
Owner

vitiral commented Dec 7, 2017

my original use case for this was to handle variables in artifact. However that proved to add too much complexity so variables are all but dropped.

However, I still think this is a generally useful library and am happy to accept PRs.

@vitiral
Copy link
Owner

vitiral commented Dec 7, 2017

Let me see if I can get a travis instance up and running, which will make things a lot easier for me.

@ghost
Copy link
Author

ghost commented Dec 7, 2017

Sure no problem 👍

@vitiral
Copy link
Owner

vitiral commented Dec 7, 2017

@seemyvest Is this code almost identical to the i64 method? If so, I think I would like to:

  • convert i64 into a genric macro that can generate these
  • just generate all the implementations using the macro

If you do that work, testing them all should be pretty easy, as the tests will be identical for most of them.

@vitiral
Copy link
Owner

vitiral commented Dec 8, 2017

The travis changes have been pushed. Please rebase and repush.

Let me know if you like the design choice of using generating macros and unifying the tests.

@ghost
Copy link
Author

ghost commented Dec 8, 2017

Yeah that's better than writing essentially the same code, I'm happy to do that, I'll just need to look into rust macros a bit more

@ghost ghost mentioned this pull request Dec 13, 2017
@vitiral
Copy link
Owner

vitiral commented Dec 13, 2017

closing in favor of #15

@vitiral vitiral closed this Dec 13, 2017
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.

1 participant