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

Create a separate crate for workunit_store #7855

Merged

Conversation

Projects
None yet
3 participants
@patliu85
Copy link
Contributor

commented Jun 5, 2019

We are decoupling Session from WorkUnit related structure.

  • Create workunit_store crate
  • Extract WorkUnit and WorkUnitStore traits from Session
  • Implement WorkUnitStore trait for Session
@illicitonion
Copy link
Contributor

left a comment

Looks good :) Just some superficial minor changes

@@ -75,6 +77,7 @@ use crate::handles::Handle;
use crate::scheduler::{ExecutionRequest, RootResult, Scheduler, Session};
use crate::tasks::{Rule, Tasks};
use crate::types::Types;
use crate::workunit_store::WorkUnitStore;

This comment has been minimized.

Copy link
@illicitonion

illicitonion Jun 5, 2019

Contributor
Suggested change
use crate::workunit_store::WorkUnitStore;
use workunit_store::WorkUnitStore;

We generally use crate:: to point at things which are defined in this crate, whereas workunit_store is an external crate.

@@ -51,6 +51,8 @@ use log;

use tar_api;

use workunit_store;

This comment has been minimized.

Copy link
@illicitonion

illicitonion Jun 5, 2019

Contributor

I think this line (and the newline after it) can be removed after my suggestion below.

@@ -19,6 +19,7 @@ use crate::core::{throw, Failure, Key, Params, TypeId, Value};
use crate::externs;
use crate::selectors;
use crate::tasks::{self, Intrinsic, Rule};
use crate::workunit_store::{WorkUnit, WorkUnitStore};

This comment has been minimized.

Copy link
@illicitonion

illicitonion Jun 5, 2019

Contributor
Suggested change
use crate::workunit_store::{WorkUnit, WorkUnitStore};
use workunit_store::{WorkUnit, WorkUnitStore};
@@ -13,6 +13,7 @@ use futures::future::{self, Future};
use crate::context::{Context, Core};
use crate::core::{Failure, Params, TypeId, Value};
use crate::nodes::{NodeKey, Select, Tracer, Visualizer};
use crate::workunit_store::{WorkUnit, WorkUnitStore};

This comment has been minimized.

Copy link
@illicitonion

illicitonion Jun 5, 2019

Contributor
Suggested change
use crate::workunit_store::{WorkUnit, WorkUnitStore};
use workunit_store::{WorkUnit, WorkUnitStore};

patliu85 added some commits Jun 5, 2019

@illicitonion illicitonion merged commit 8faa1e9 into pantsbuild:master Jun 7, 2019

1 check was pending

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details

@illicitonion illicitonion deleted the twitter:pliu/v2_workunit_store_trait branch Jun 7, 2019

cattibrie added a commit to cattibrie/pants that referenced this pull request Jun 19, 2019

Create a separate crate for workunit_store (pantsbuild#7855)
We are decoupling `Session` from `WorkUnit` related structure.

 - Create workunit_store crate
 - Extract WorkUnit and WorkUnitStore traits from `Session`
 - Implement `WorkUnitStore` trait for `Session`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.