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 times and expires to Entry and Group #38

Merged
merged 2 commits into from
Apr 23, 2021

Conversation

jgrussell
Copy link
Contributor

This adds the ExpiryTime entry from the XML; and, I think it sets the stage for quick and easy additions of other XML timestamp fields as needed/requested.

It also adds and the Expires flag from the XML to both Group and Entry since ExpiryTime is almost meaningless without this flag.

Please let me know if you want me to make any changes, anything from code tweaks to overall approach; or, feel free to edit my fork directly.

Once this is merged, #29 will be complete in my mind.

Thank you again!

Copy link
Owner

@sseemayer sseemayer left a comment

Choose a reason for hiding this comment

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

Beautiful work on this PR 👍 ! I have some minor comments but this is already on a great way to getting merged.

src/xml_parse.rs Show resolved Hide resolved
src/db.rs Show resolved Hide resolved
src/db.rs Show resolved Hide resolved
src/db.rs Outdated Show resolved Hide resolved
src/xml_parse.rs Outdated Show resolved Hide resolved
@jgrussell
Copy link
Contributor Author

@sseemayer

Thank you for the compliment and the quick review.

Please take a look at these two conversations and comment further when you have time:
parse_xml_timestamp
pub times: HashMap

I will work on these changes over the weekend and resolve the associated conversations as I progress:
Add get_expiry_time method.
Suggested get_time method changes.
Suggested Vec into [u8; 8] changes.

@sseemayer
Copy link
Owner

Thanks also to you for the elaborations! I've added some further comments on the parse_xml_timestamp and pub times: HashMap discussions.

@sseemayer sseemayer merged commit 593a3ab into sseemayer:master Apr 23, 2021
@sseemayer
Copy link
Owner

Wonderful! 🎉 Thanks a lot for the nice contribution.

@jgrussell
Copy link
Contributor Author

Thank you again for the quick PR review and detailed feedback.

sseemayer pushed a commit that referenced this pull request Dec 29, 2022
* Add times and expires to Entry and Group

* Updates from initial PR discussions

Co-authored-by: John G. Russell <jgrgit@hiwt.com>
sseemayer pushed a commit that referenced this pull request Dec 30, 2022
* Add times and expires to Entry and Group

* Updates from initial PR discussions

Co-authored-by: John G. Russell <jgrgit@hiwt.com>
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.

None yet

2 participants