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.

Sign up for GitHub

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

Jump to bottom

refactor(workspace): move types to gprimitives crate #3919

Merged
merged 74 commits into from
May 29, 2024
Merged

refactor(workspace): move types to gprimitives crate #3919

merged 74 commits into from
May 29, 2024

Conversation

StackOverflowExcept1on
Copy link
Member

@StackOverflowExcept1on StackOverflowExcept1on commented Apr 24, 2024 •
edited
Loading

Resolves #2944

Motivation: Mainly this PR is useful for I/O crates. If user wants to declare I/O types that depend on gstd, gprimitives = { workspace = true, features = ["codec"] } should be used instead.

We also encountered linking issue on Windows at hackathon. This is because MSVC linker requires declaration of all imported functions from gsys.

@StackOverflowExcept1on StackOverflowExcept1on added A0-pleasereview PR is ready to be reviewed by the team C2-refactoring Refactoring proposal labels Apr 24, 2024
@StackOverflowExcept1on StackOverflowExcept1on added the B2-breaking-apis A breaking change of which all stakeholders must be warned label Apr 24, 2024
breathx
breathx requested changes Apr 26, 2024
View reviewed changes
Copy link
Member

@breathx breathx left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Need to sync on impl details since this isn't resolve initial issue (motivation of implementing separate crate)

UPD: see #2944

@breathx breathx added A3-gotissues PR occurred to have issues after the review and removed A0-pleasereview PR is ready to be reviewed by the team labels Apr 26, 2024
@StackOverflowExcept1on StackOverflowExcept1on added the A4-insubstantial Not too important PR label Apr 27, 2024
@StackOverflowExcept1on StackOverflowExcept1on changed the title refactor(gcore, gstd): move types to gcore-primitives crate refactor(workspace): move types to gprimitives crate Apr 27, 2024
@StackOverflowExcept1on StackOverflowExcept1on added A0-pleasereview PR is ready to be reviewed by the team and removed A3-gotissues PR occurred to have issues after the review labels May 19, 2024
breathx
breathx approved these changes May 26, 2024
View reviewed changes
gstd/src/msg/encoded.rs Outdated Show resolved Hide resolved
gprimitives/src/lib.rs Outdated Show resolved Hide resolved
gprimitives/src/lib.rs Outdated Show resolved Hide resolved
gprimitives/src/lib.rs Show resolved Hide resolved
gcore/src/utils.rs Outdated Show resolved Hide resolved
gprimitives/src/lib.rs Show resolved Hide resolved
core/src/ids.rs Outdated Show resolved Hide resolved
@breathx
Copy link
Member

breathx commented May 26, 2024

Resolve comments above and good to go from my side 👍🏻

ptal once again @ark0f

@breathx breathx added the A2-mergeoncegreen PR is ready to merge after CI passes label May 26, 2024
ark0f
ark0f approved these changes May 26, 2024
View reviewed changes
Copy link
Member

@ark0f ark0f left a comment

Choose a reason for hiding this comment

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

Awesome 👌

@breathx
Copy link
Member

breathx commented May 29, 2024

Good to merge from my side @StackOverflowExcept1on

@breathx breathx removed the A0-pleasereview PR is ready to be reviewed by the team label May 29, 2024
@StackOverflowExcept1on StackOverflowExcept1on merged commit cb6dae9 into master May 29, 2024
10 checks passed
@StackOverflowExcept1on StackOverflowExcept1on deleted the av/gcore-primitives branch May 29, 2024 16:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ark0f ark0f ark0f approved these changes

@breathx breathx breathx approved these changes

@DennisInSky DennisInSky Awaiting requested review from DennisInSky

Assignees
No one assigned
Labels
A2-mergeoncegreen PR is ready to merge after CI passes B2-breaking-apis A breaking change of which all stakeholders must be warned C2-refactoring Refactoring proposal
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Introduce gstd-primitives crate
4 participants
@StackOverflowExcept1on @DennisInSky @breathx @ark0f