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

Bump uuid from 0.8.0 -> 1.0.0 #27

Merged
merged 4 commits into from
Jul 13, 2022
Merged

Bump uuid from 0.8.0 -> 1.0.0 #27

merged 4 commits into from
Jul 13, 2022

Conversation

kylewillmon
Copy link
Contributor

This patch bumps the version of the uuid crate to 1.0.0. This is a
breaking change because uuid is used in the public interface of this
crate.

Ref: phylum-dev/cli#326

This patch bumps the version of the uuid crate to 1.0.0. This is a
breaking change because uuid is used in the public interface of this
crate.
@cd-work
Copy link
Contributor

cd-work commented Apr 29, 2022

I just noticed this, but is there a reason why phylum-types does not have any CI setup? Especially for PRs like these that wouldn't need any in-depth testing that would make reviews much easier.

@kylewillmon
Copy link
Contributor Author

I just noticed this, but is there a reason why phylum-types does not have any CI setup? Especially for PRs like these that wouldn't need any in-depth testing that would make reviews much easier.

Just because nobody has set it up yet, I believe.

@cd-work
Copy link
Contributor

cd-work commented Apr 29, 2022

I've opened #28 just in case I don't get to it today so it doesn't get lost over the weekend.

Copy link
Contributor

@cd-work cd-work left a comment

Choose a reason for hiding this comment

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

Should be good after a rebase.

cd-work
cd-work previously approved these changes May 2, 2022
@kylewillmon
Copy link
Contributor Author

The uuid crate is heavily used in api, so I will wait for approval from someone in @phylum-dev/user-components before merging this.

It looks like other dependencies for api are making this jump though, so it will be hitting that repo soon with or without this PR.

@matt-phylum
Copy link
Contributor

I tried to compile api with this change. Rocket and sqlx are bringing in the old uuid and there are conflicts at least for sqlx. Relevant sqlx PR: launchbadge/sqlx#1821

@DanielJoyce
Copy link
Contributor

This needs to hold off till we can update API for now.

kylewillmon added a commit to phylum-dev/cli that referenced this pull request May 11, 2022
It turns out that we don't actually need this as a direct dependency
because `phylum_types` has type aliases for all of this.

Doing it this way has pros and cons. Maybe someday we'll decide that the
aliases in `phylum_types` don't actually make sense or that they should
actually use the newtype pattern. But for now, they exist, so we might
as well use them.

As an nice bonus, this means we won't have to worry about coordinating
our version bump of the uuid crate with phylum-dev/phylum-types#27. Once
that is merged, we will automatically be upgrade (after the next cargo
update)
kylewillmon added a commit to phylum-dev/cli that referenced this pull request May 11, 2022
It turns out that we don't actually need this as a direct dependency
because `phylum_types` has type aliases for all of this.

Doing it this way has pros and cons. Maybe someday we'll decide that the
aliases in `phylum_types` don't actually make sense or that they should
actually use the newtype pattern. But for now, they exist, so we might
as well use them.

As an nice bonus, this means we won't have to worry about coordinating
our version bump of the uuid crate with phylum-dev/phylum-types#27. Once
that is merged, we will automatically be upgrade (after the next cargo
update)
kylewillmon added a commit to phylum-dev/cli that referenced this pull request May 11, 2022
It turns out that we don't actually need this as a direct dependency
because `phylum_types` has type aliases for all of this.

Doing it this way has pros and cons. Maybe someday we'll decide that the
aliases in `phylum_types` don't actually make sense or that they should
actually use the newtype pattern. But for now, they exist, so we might
as well use them.

As an nice bonus, this means we won't have to worry about coordinating
our version bump of the uuid crate with phylum-dev/phylum-types#27. Once
that is merged, we will automatically be upgrade (after the next cargo
update)
@matt-phylum matt-phylum merged commit ac678c1 into development Jul 13, 2022
@matt-phylum matt-phylum deleted the uuid-bump branch July 13, 2022 18:53
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.

5 participants