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

Add instance status column. Implements #572 #600

Merged
merged 2 commits into from
Dec 29, 2017
Merged

Conversation

dmfs
Copy link
Owner

@dmfs dmfs commented Dec 27, 2017

This commit adds an instance status column which indicates which instance of a task is the next one to complete. This allows to filter out the instances which come after the next one.

This commit adds an instance status column which indicates which instance of a task is the next one to complete. This allows to filter out the instances which come after the next one.
@dmfs dmfs requested a review from lemonboston December 27, 2017 23:48
Copy link
Contributor

@lemonboston lemonboston left a comment

Choose a reason for hiding this comment

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

Besides those small things, it looks good to me.

@@ -237,7 +237,8 @@
+ TaskContract.Instances.INSTANCE_START_SORTING + " INTEGER, "
+ TaskContract.Instances.INSTANCE_DUE_SORTING + " INTEGER, "
+ TaskContract.Instances.INSTANCE_DURATION + " INTEGER, "
+ TaskContract.Instances.INSTANCE_ORIGINAL_TIME + " INTEGER DEFAULT 0);";
+ TaskContract.Instances.INSTANCE_ORIGINAL_TIME + " INTEGER DEFAULT 0,"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is a space needed here after DEFAULT 0,?

Copy link
Owner Author

Choose a reason for hiding this comment

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

no, technically it's not required



/**
* Create task with start and due, check datetime values including generated duration.
Copy link
Contributor

Choose a reason for hiding this comment

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

Javadoc stayed copy-pasted here.

Copy link
Owner Author

Choose a reason for hiding this comment

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

fixed



/**
* A {@link Single} of date and time {@link ContentValues} of an instance.
Copy link
Contributor

Choose a reason for hiding this comment

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

Not updated, copy-pasted javadoc here.

Copy link
Owner Author

Choose a reason for hiding this comment

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

fixed

@dmfs
Copy link
Owner Author

dmfs commented Dec 28, 2017

@lemonboston please re-review.

Copy link
Contributor

@lemonboston lemonboston left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@dmfs dmfs merged commit d0ff43e into master Dec 29, 2017
@dmfs dmfs deleted the stories/572-is-upcoming branch December 29, 2017 12:24
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.

2 participants