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

Improve 'compilation required' check by checking source file dependencies #312

Merged
merged 3 commits into from
May 30, 2017

Conversation

Krusen
Copy link
Contributor

@Krusen Krusen commented May 24, 2017

This recursively check the last write time of all the files for which the input file depends on (e.g. @import etc.) instead of only the input file.

Without this there can be situations where the input files has not been changed but dependent files have been changed outside Visual Studio (e.g. by source control) and would not be recompiled. This can often happen with the "build on compile" feature after switching branches or fetching updates.

@Krusen Krusen force-pushed the compilation-required-fix branch from 0bfe7fd to 7aa5068 Compare May 24, 2017 19:47
@Krusen Krusen force-pushed the compilation-required-fix branch from 7aa5068 to c965f5c Compare May 24, 2017 19:49
@Krusen
Copy link
Contributor Author

Krusen commented May 25, 2017

I've added some unit tests to show that it actually works.

@madskristensen
Copy link
Owner

Did you see my comment on the File.Exist check?

@Krusen
Copy link
Contributor Author

Krusen commented May 26, 2017

No, I don't see any comments at all.

@Krusen
Copy link
Contributor Author

Krusen commented May 30, 2017

@madskristensen If you will point it out again I'll get it fixed :)

foreach (var file in dependencies[key].DependentOn.ToArray())
{
var fileInfo = new FileInfo(file);
if (fileInfo.LastWriteTimeUtc > output.LastWriteTimeUtc)
Copy link
Owner

Choose a reason for hiding this comment

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

Should there be a check for fileInfo.Exist before calling the LastWriteTimeUtc properties?

Copy link
Contributor Author

@Krusen Krusen May 30, 2017

Choose a reason for hiding this comment

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

According to the documentation if the file doesn't exist it will return 12:00 midnight, January 1, 1601 A.D. (C.E.) Coordinated Universal Time (UTC), but it would of course be good practice to just check if it exists and make the intention more clear.

I'll add the check.

@madskristensen madskristensen merged commit b565092 into madskristensen:master May 30, 2017
@madskristensen
Copy link
Owner

Thanks

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