gh-uuid-rs/uuid
Watch
1
0
Fork
You've already forked uuid
 0
Code Issues 10 Pull requests 2 Releases Activity

Use cfg-if to create logical feature gate implementations #167

Merged
kinggoesgaming merged 6 commits from cfg_if into master 2018-03-14 18:26:16 -05:00
Conversation 12 Commits 6 Files changed 4 +111 -69
kinggoesgaming commented 2018-03-11 18:58:34 -05:00 (Migrated from github.com)

I'm submitting a ...

  • bug fix
  • feature enhancement
  • deprecation or removal
  • refactor

Description

uuid now adds cfg-if as a dependency. With this, #[cfg(..)] implementations are easier to understand, as they can be handled in a more logical view because the macro allows us to if statements to make the checks more rustic at the source level

Motivation

Understanding feature gate implementation becomes easier.

Tests

The CI builds passing should be plenty.

Related Issue(s)

#147

<!-- As we are working towards a stable version of uuid, we require that you open an issue, before submitting a pull request. If the pull request is imcomplete, prepend the Title with WIP: --> **I'm submitting a ...** - [ ] bug fix - [ ] feature enhancement - [ ] deprecation or removal - [x] refactor # Description `uuid` now adds `cfg-if` as a dependency. With this, `#[cfg(..)]` implementations are easier to understand, as they can be handled in a more logical view because the macro allows us to `if` statements to make the checks more rustic at the source level # Motivation Understanding feature gate implementation becomes easier. # Tests The CI builds passing should be plenty. # Related Issue(s) #147
radix (Migrated from github.com) reviewed 2018-03-11 18:58:34 -05:00
kinggoesgaming commented 2018-03-11 18:59:39 -05:00 (Migrated from github.com)

question: should I put various feature gated constructor new functions behind cfg_if` macros aswell or should we wait until a bigger refactor comes along?

question: should I put various feature gated constructor `new` functions behind cfg_if` macros aswell or should we wait until a bigger refactor comes along?
kinggoesgaming commented 2018-03-13 19:57:28 -05:00 (Migrated from github.com)

@uuid-rs/uuid I think this is logical enough for this PR. Should be ready for review and merge

@uuid-rs/uuid I think this is logical enough for this PR. Should be ready for review and merge
KodrAus (Migrated from github.com) reviewed 2018-03-13 20:06:56 -05:00
KodrAus (Migrated from github.com) left a comment

This looks a lot easier to follow! Just had a quick scan through on mobile

This looks a lot easier to follow! Just had a quick scan through on mobile
src/lib.rs
@ -146,0 +173,4 @@
mod serde_support;
}
}
KodrAus (Migrated from github.com) commented 2018-03-13 20:05:21 -05:00

Maybe we could reduce some duplicated nesting here by conditionally using core as std:

if #[cfg(not(feature = "std"))] {
    use core as std;
}

use std::fmt;
use std::str;

...

What do you think?

Maybe we could reduce some duplicated nesting here by conditionally using `core as std`: ```rust if #[cfg(not(feature = "std"))] { use core as std; } use std::fmt; use std::str; ... ``` What do you think?
kinggoesgaming (Migrated from github.com) reviewed 2018-03-13 20:12:03 -05:00
src/lib.rs
@ -146,0 +173,4 @@
mod serde_support;
}
}
kinggoesgaming (Migrated from github.com) commented 2018-03-13 20:12:03 -05:00

This PR was intended to switch to cfg_if. Another PR should make the that part more readable later... which I think I will have ready soon

This PR was intended to switch to `cfg_if`. Another PR should make the that part more readable later... which I think I will have ready soon
Dylan-DPC-zz (Migrated from github.com) approved these changes 2018-03-14 16:22:35 -05:00
Dylan-DPC-zz commented 2018-03-14 16:22:51 -05:00 (Migrated from github.com)

bors: r+

bors: r+
bors[bot] commented 2018-03-14 16:29:42 -05:00 (Migrated from github.com)

Build failed

# Build failed * [continuous-integration/travis-ci/push](https://travis-ci.org/uuid-rs/uuid/builds/353549819?utm_source=github_status&utm_medium=notification)
kinggoesgaming commented 2018-03-14 18:08:54 -05:00 (Migrated from github.com)

clippy was outdated, cache cleared

bors r+

clippy was outdated, cache cleared bors r+
bors[bot] commented 2018-03-14 18:13:31 -05:00 (Migrated from github.com)

Build failed

# Build failed * [continuous-integration/travis-ci/push](https://travis-ci.org/uuid-rs/uuid/builds/353588853?utm_source=github_status&utm_medium=notification)
kinggoesgaming commented 2018-03-14 18:19:25 -05:00 (Migrated from github.com)

again! wrong cache cleared :(

bors r+

again! wrong cache cleared :( bors r+
bors[bot] commented 2018-03-14 18:26:15 -05:00 (Migrated from github.com)

Build succeeded

# Build succeeded * [continuous-integration/travis-ci/push](https://travis-ci.org/uuid-rs/uuid/builds/353592188?utm_source=github_status&utm_medium=notification) * [continuous-integration/appveyor/branch](https://ci.appveyor.com/project/KodrAus/uuid/build/1.0.214)
Sign in to join this conversation.
Reviewers
No reviewers
radix
Dylan-DPC-zz
Labels
Clear labels   
breaking
This causes a breaking change and will be merged to breaking branch.

  
bug

   
dependencies
Pull requests that update a dependency file

  
good first issue

   
help wanted

   
unstable
Tracking issue for an unstable optional dependency

No labels
breaking
bug
dependencies
good first issue
help wanted
unstable
Milestone
Clear milestone
No items
No milestone
0.6.2
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
gh-uuid-rs/uuid!167
No description provided.