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 needsRefresh for multiple targets #65

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

iffy
Copy link

@iffy iffy commented Feb 11, 2019

For the case when a build step produces multiple targets. For my case, using tsc to compile TypeScript files to JavaScript.

@iffy iffy changed the title Add needsRefresh for mutliple targets Add needsRefresh for multiple targets Feb 14, 2019
@iffy
Copy link
Author

iffy commented Feb 14, 2019

@yglukhov Does this look good to you?

nakelib.nim Outdated
@@ -193,6 +193,38 @@ template withDir*(dir: string; body: untyped): untyped =
cd(curDir)


proc needsRefresh*(targets: seq[string], src: varargs[string]): bool {.raises: [OSError].} =
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd better make both args openarray, for maximum flexibility.

nakelib.nim Outdated
var minTargetTime: float = -1
for target in targets:
try:
let targetTime = toSeconds(getLastModificationTime(target))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like with toSeconds we lose precision that might be important. Can we do better?

Copy link
Author

Choose a reason for hiding this comment

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

Probably? I just copied the existing implementation (trying not to change too many things at once)

Copy link
Author

Choose a reason for hiding this comment

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

@yglukhov Are you thinking maybe toWinTime()?

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