Skip to content

Move generated code to directories with _gen suffix #676

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

Merged
merged 2 commits into from
Dec 4, 2018

Conversation

sejr
Copy link

@sejr sejr commented Nov 27, 2018

This PR addresses issue #602.

cc @andreeaflorescu

@raduweiss
Copy link
Contributor

@sejr you might have noticed that you can't access the test fail error. Sorry about that! We're still working to make those logs public. Until then, the integration tests we run are exactly the ones you can run with tools/devtool test. See the testing section of the getting started guide for details.

@dianpopa
Copy link
Contributor

@sejr your PR does not pass the build as there are dependencies in the code where you did not use the newly updated name of the crates. For example, this file still looks for kvm_sys. Additionally, after "toml" dependencies have been updated, you will also need to update the use scenarios: e.g. use kvm_sys::*; from this file.

@sejr
Copy link
Author

sejr commented Nov 28, 2018

@raduweiss Thank you for clarifying!
@dianpopa Oops. Fixed :)

@andreeaflorescu
Copy link
Member

Hey, the changes look good, but can you please squash your commits? We want each individual commit on the master branch to build and pass the unit tests.

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

@sejr changes look good, thanks for the PR! Please address @andreeaflorescu's comment and squash your commits.

@acatangiu acatangiu self-assigned this Dec 4, 2018
@sejr sejr force-pushed the move-gen-code branch 2 times, most recently from 668cd66 to 632824b Compare December 4, 2018 19:28
@sejr
Copy link
Author

sejr commented Dec 4, 2018

@acatangiu @andreeaflorescu Commits have been squashed.

pub const KVM_SYSTEM_EVENT_SHUTDOWN: ::std::os::raw::c_uint = 1;
pub const KVM_SYSTEM_EVENT_RESET: ::std::os::raw::c_uint = 2;
pub const KVM_SYSTEM_EVENT_CRASH: ::std::os::raw::c_uint = 3;
pub const kvm_genTEM_EVENT_SHUTDOWN: ::std::os::raw::c_uint = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like these lines were affected by some sort of find/replace collateral damage.

Copy link
Author

Choose a reason for hiding this comment

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

@alexandruag Fixed.

Copy link
Contributor

@acatangiu acatangiu left a comment

Choose a reason for hiding this comment

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

pub const KVM_SYSTEM_EVENT_SHUTDOWN: ::std::os::raw::c_uint = 1;
pub const KVM_SYSTEM_EVENT_RESET: ::std::os::raw::c_uint = 2;
pub const KVM_SYSTEM_EVENT_CRASH: ::std::os::raw::c_uint = 3;
pub const kvm_genTEM_EVENT_SHUTDOWN: ::std::os::raw::c_uint = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't look right.

Signed-off-by: Sam Jackson <[email protected]>

Changed references to *_sys crates with corresponding *_gen names

Fixed minor typo after find+replace
@acatangiu acatangiu merged commit a8961ce into firecracker-microvm:master Dec 4, 2018
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.

6 participants