Description

Adds const_ft(0.1.2) as a dependency
Refactors code to use the dependency

Motivation

Code looks cleaner and avoids const(feature) & const(not-feature) repetition

Tests

All tests pass, none added

**I'm submitting a(n)** refactor # Description Adds const_ft(0.1.2) as a dependency Refactors code to use the dependency # Motivation Code looks cleaner and avoids const(feature) & const(not-feature) repetition # Tests All tests pass, none added

todo[bot] commented

2018-08-20 15:11:27 -05:00

(Migrated from github.com)

unused function

github.com/uuid-rs/uuid@d347d0638f/src/parser/mod.rs (L115-L120)

This comment was generated by todo based on a `TODO` comment in `d347d0638f` in #301. cc @uuid-rs.

## unused function https://github.com/uuid-rs/uuid/blob/d347d0638f3e95b1b05cfbb6a7841f7a26e51be1/src/parser/mod.rs#L115-L120 --- ###### This comment was generated by [todo](https://todo.jasonet.co) based on a `TODO` comment in d347d0638f3e95b1b05cfbb6a7841f7a26e51be1 in #301. cc @uuid-rs.

KodrAus commented

2018-08-20 16:29:18 -05:00

(Migrated from github.com)

Hmm, I think we should be really mindful about introducing new dependencies. Do we need to use an external crate for const functions? Using a macro seems nice, but it could easily just live here in uuid.

Hmm, I think we should be really mindful about introducing new dependencies. Do we need to use an external crate for const functions? Using a macro seems nice, but it could easily just live here in `uuid`.

kinggoesgaming commented

2018-08-22 20:12:31 -05:00

(Migrated from github.com)

As there some problems with the macro in itself, I think its best to not depend on it either way until they are polished out

Dylan-DPC-zz commented

2018-08-23 03:42:10 -05:00

(Migrated from github.com)

@kodrAus it doesn't make much of a difference since it's my own crate. Though currently there are some issues with the macro so might postpone this

KodrAus commented

2018-08-23 04:11:20 -05:00

(Migrated from github.com)

@Dylan-DPC there are a few differences worth considering. The first is compile times where additional dependencies mean more units of compilation, more metadata, more network, so they'll regress slightly. The second is in mental overhead, where there's an additional non-local piece of infrastructure new contributors need to understand in order to contribute.

I'm not suggesting those are hard blockers, but we should definitely consider them.

@Dylan-DPC there are a few differences worth considering. The first is compile times where additional dependencies mean more units of compilation, more metadata, more network, so they'll regress slightly. The second is in mental overhead, where there's an additional non-local piece of infrastructure new contributors need to understand in order to contribute. I'm not suggesting those are hard blockers, but we should definitely consider them.

👍 1

Dylan-DPC-zz commented

2018-08-23 15:43:21 -05:00

(Migrated from github.com)

@KodrAus I understand your point and it was a reason why i wasn't so keen on cfg_if. One of the main reasons of creating a new crate is that it wasn't related to uuids and can be something of greater use to other projects as well.

Turns out there is still some more stuff to be done with the macros so closing the PR now and we can reconsider it later.

@KodrAus I understand your point and it was a reason why i wasn't so keen on `cfg_if`. One of the main reasons of creating a new crate is that it wasn't related to uuids and can be something of greater use to other projects as well. Turns out there is still some more stuff to be done with the macros so closing the PR now and we can reconsider it later.

Pull request closed

This pull request cannot be reopened because the branch was deleted.

Rows
Columns

refactored code to use const_ft #301

Description

Motivation

Tests

unused function

This comment was generated by todo based on a TODO comment in d347d0638f in #301. cc @uuid-rs.

Pull request closed

This comment was generated by todo based on a `TODO` comment in `d347d0638f` in #301. cc @uuid-rs.