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

Use existing file metadata for created and modified dates #1217

Merged
merged 3 commits into from
Dec 16, 2017
Merged

Use existing file metadata for created and modified dates #1217

merged 3 commits into from
Dec 16, 2017

Conversation

SCdF
Copy link
Contributor

@SCdF SCdF commented Dec 1, 2017

NB: this definitely works on OSX. Other operating systems may have
slightly different interpretations of (eg) birthtime.

See: https://nodejs.org/api/fs.html#fs_class_fs_stats

@kazup01 kazup01 requested a review from sota1235 December 2, 2017 02:24
@kazup01 kazup01 added the awaiting review ❇️ Pull request is awaiting a review. label Dec 7, 2017
@sota1235
Copy link
Contributor

@kazup01 Could you check whether this PR works on windows? If it works, it's ok

@sota1235 sota1235 requested review from kazup01 and removed request for sota1235 December 13, 2017 15:19
@kazup01
Copy link
Member

kazup01 commented Dec 13, 2017

@sota1235 Sure. I will check it tomorrow.

@sota1235 sota1235 removed their assignment Dec 13, 2017
@kazup01 kazup01 added awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. and removed awaiting review ❇️ Pull request is awaiting a review. labels Dec 14, 2017
@kazup01
Copy link
Member

kazup01 commented Dec 14, 2017

Hi @SCdF ,

An error occurs in the following procedure.

  • Select note in the note list
  • Move note to another folder with drag and drop
  • The number of notes is not counted in the number next to folder name
  • An error occurs when selecting another note.

Attach gif and errors in the console.

dec-14-0029 13-16-21

screen shot 0029-12-14 at 1 16 59 pm

@SCdF
Use existing file metadata for created and modified dates
9032a1d 
NB: this definintely works on OSX. Other operating systems may have
slightly different interpretations of birthtime.

See: https://nodejs.org/api/fs.html#fs_class_fs_stats
@SCdF
Not used to this no semi colon thing
b1063eb
@SCdF
Reorder Object.assign
745d250 
 - Respects the dates that may be in input over default dates
 - Respects key and storage over what might be in input
@SCdF SCdF force-pushed the use-file-metadata-on-import branch from 93669ea to 745d250 Compare December 14, 2017 13:16
@SCdF
Copy link
Contributor Author

SCdF commented Dec 14, 2017

Nice catch! I didn't realise createNote is involved when you move notes. This has happened because I flipped the order of an Object assign to prioritise calculated dates over default "now" ones.

I've rejigged it so that it this is no longer a problem.

kazup01
kazup01 approved these changes Dec 16, 2017
View reviewed changes
Copy link
Member

@kazup01 kazup01 left a comment

Choose a reason for hiding this comment

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

LGTM

@kazup01 kazup01 merged commit 0cc9f00 into BoostIO:master Dec 16, 2017
@kazup01
Copy link
Member

kazup01 commented Dec 16, 2017

Merged. Thanks @SCdF ;)

@kazup01 kazup01 added next release (v0.8.19) and removed awaiting changes 🖊️ Pull request has been reviewed, but contributor needs to make changes. Next Release labels Dec 16, 2017
@kohei-takata kohei-takata mentioned this pull request Dec 23, 2017
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kazup01 kazup01 kazup01 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@SCdF @sota1235 @kazup01 @BoostnoteBot