Skip to content
This repository has been archived by the owner on Nov 1, 2021. It is now read-only.

add full uuid macro #1

Merged
merged 13 commits into from
May 26, 2018
Merged

add full uuid macro #1

merged 13 commits into from
May 26, 2018

Conversation

mokeyish
Copy link
Member

@mokeyish mokeyish commented May 4, 2018

It depend on ref const Uuid::from_uuid_bytes

The example

extern crate uuid;
extern crate uuid_macro;

use uuid::Uuid;

// if you want to create const uuid value,just enable nightly feature.
const CONST_UUID: Uuid = uuid!("936DA01F9ABD4d9d80C702AF85C822A8");

fn main() {
    let my_uuid = uuid!("936DA01F9ABD4d9d80C702AF85C822A8")
    println!("Parsed a version {} UUID.", my_uuid.get_version_num());
    println!("{}", my_uuid);
}

src/lib.rs Outdated
@@ -1,101 +1,57 @@
#![recursion_limit="128"]
extern crate uuid;
pub use uuid::Uuid;
Copy link
Member

Choose a reason for hiding this comment

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

we can't have a pub export here...

src/lib.rs Outdated
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

newline

Copy link
Member

Choose a reason for hiding this comment

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

newlines will be fixed by rustfmt so not a worry

Cargo.toml Outdated

[features]
default = []
nightly = []
Copy link
Member

Choose a reason for hiding this comment

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

newline

src/lib.rs Outdated

#[cfg(feature = "nightly")]
Copy link
Member

Choose a reason for hiding this comment

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

just use #[cfg(nightly)]

Copy link
Member Author

Choose a reason for hiding this comment

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

@kinggoesgaming Are you sure #[cfg(nightly)]really working? I create an new hello-word project with cargo new hello-world --bin, and modify main.rs :

fn main() {
    test();
}

#[cfg(nightly)]
fn test() {
    println!("nightly, Hello, world!");
}

#[cfg(not(nightly))]
fn test() {
    println!("not nightly, Hello, world!");
}

Run with command cargo +nightly run,It still prints not nightly, Hello, world!;

Copy link
Member

Choose a reason for hiding this comment

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

@kinggoesgaming yah he is right. I reproduced it here and I'm getting the same output

Copy link
Member

Choose a reason for hiding this comment

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

I guess it works only when you have a mod foo or extern crate.

Copy link
Member

Choose a reason for hiding this comment

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

Something funky is definitely going on... Imma fiddle around today on this

Copy link
Member

@kinggoesgaming kinggoesgaming May 7, 2018

Choose a reason for hiding this comment

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

alright after going through the mighty google, I seems I was wrong. #[cfg(nightly)] was discussed at one point, however no resolution... This includes the mod foo

This needs resolution in a way that user does not have to explicitly provide the nightly feature gate. I will probably use a build script in combination with rustc_version for this

Copy link
Member

Choose a reason for hiding this comment

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

@mokeyish see uuid-rs/uuid#220

I think the same setup can be added here (perhaps as a separate PR that is merged before this PR)

Copy link
Member

Choose a reason for hiding this comment

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

@kinggoesgaming what's the issue with providing an explicit feature in order to enable an unstable nightly Rust feature?

Copy link
Member

Choose a reason for hiding this comment

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

proceed with the #[cfg(feature = "nightly")]

src/lib.rs Outdated
{$literal:expr} => {
{
const BYTES: [u8; 16] = uuid_parts! {$literal};
$crate::Uuid::from_uuid_bytes(BYTES)
Copy link
Member

@kinggoesgaming kinggoesgaming May 7, 2018

Choose a reason for hiding this comment

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

As it is clear that people need to depend on uuid, instead use:

{
  use uuid;
  uuid::Uuid::from_uuid_bytes(BYTES)
}

Cargo.toml Outdated
[dependencies]
uuid_macro_impl = { path = "./uuid_macro_impl"}
proc-macro-hack = { version = "0.4"}
uuid = { git = "https://github.com/mokeyish/uuid.git", branch = "master" }
Copy link
Member

Choose a reason for hiding this comment

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

why depend on the fork?

Copy link
Member Author

Choose a reason for hiding this comment

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

@Dylan-DPC Because this have't approved yet. Do you know why about this snapshot.
image

Copy link
Member

Choose a reason for hiding this comment

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

Yes i know. Just made that note so we remember to switch back to the crate once that is completed.

Copy link
Member

@kinggoesgaming kinggoesgaming May 11, 2018

Choose a reason for hiding this comment

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

the PR has been merged, so update this...

Note that once we get [email protected] is released we update this again... (later)

Copy link
Member Author

Choose a reason for hiding this comment

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

ok

@@ -0,0 +1,23 @@
[package]
authors = [
"Hunar Roop Kahlon <[email protected]>",
Copy link
Member

Choose a reason for hiding this comment

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

you forgot me 😆

Copy link
Member

Choose a reason for hiding this comment

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

Oops...

bors bot added a commit to uuid-rs/uuid that referenced this pull request May 11, 2018
218: Add const Uuid::from_uuid_bytes r=kinggoesgaming a=mokeyish

<!--
    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  const Uuid::from_uuid_bytes for creating const uuid value.**  [ref uuid_macro](uuid-rs/uuid_macro#1)
  - [ ] bug fix
  - [x] feature enhancement
  - [ ] deprecation or removal
  - [ ] refactor

# Description
<!-- Provide a summary of your changes in the Title above -->

# Motivation
<!-- Why is this change required -->

# Tests
<!-- How are these changes tested? -->

# Related Issue(s)
<!-- 
    As noted above, we require an issue for every PR. Please link to the issue
    here
-->


Co-authored-by: MOKEYISH <[email protected]>
Co-authored-by: YISH <[email protected]>
@kinggoesgaming
Copy link
Member

@mokeyish can you update the version of uuid?

@mokeyish
Copy link
Member Author

@kinggoesgaming The uuid create has an error.The Cargo.toml used const_fn = ["nightly"],but the /src/lib.rs used const-fn. So it worked unexpectedly.

@kinggoesgaming
Copy link
Member

Oh no that managed to slip through... @Dylan-DPC or me will get a new version released to fix that

@kinggoesgaming
Copy link
Member

@mokeyish update to 0.6.5 uuid

@mokeyish
Copy link
Member Author

bors r+

@Dylan-DPC-zz
Copy link
Member

@mokeyish you can't r+ till all the changes were approved

@Dylan-DPC-zz
Copy link
Member

Dylan-DPC-zz commented May 25, 2018

@mokeyish can you change the code as per what is requested in the review first? We can't merge till then
ignore this

@Dylan-DPC-zz
Copy link
Member

@kinggoesgaming your previous review looks outdated now. Can you dismiss it and rereview?

Cargo.toml Outdated
[dependencies]
uuid_macro_impl = { path = "./uuid_macro_impl"}
proc-macro-hack = { version = "0.4"}
uuid = { version = "0.6.5", features = ["const_fn"]}
Copy link
Member

Choose a reason for hiding this comment

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

can you add default-features = false here as well?

@kinggoesgaming
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request May 26, 2018
1: add full uuid macro r=kinggoesgaming a=mokeyish

It depend on [ref const Uuid::from_uuid_bytes](uuid-rs/uuid#218)

*The example*

```rust
extern crate uuid;
extern crate uuid_macro;

use uuid::Uuid;

// if you want to create const uuid value,just enable nightly feature.
const CONST_UUID: Uuid = uuid!("936DA01F9ABD4d9d80C702AF85C822A8");

fn main() {
    let my_uuid = uuid!("936DA01F9ABD4d9d80C702AF85C822A8")
    println!("Parsed a version {} UUID.", my_uuid.get_version_num());
    println!("{}", my_uuid);
}
```

Co-authored-by: YISH <[email protected]>
Co-authored-by: YISH <[email protected]>
Co-authored-by: mokeyish <[email protected]>
@bors
Copy link
Contributor

bors bot commented May 26, 2018

Build succeeded

@bors bors bot merged commit cbeaefc into master May 26, 2018
@kinggoesgaming kinggoesgaming deleted the feature/uuid-macro branch May 26, 2018 00:59
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.

4 participants