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

Monitor against individual file timestamp changes instead of System/currentTimeMillis #5

Closed
wants to merge 7 commits into from

Conversation

CmdrDats
Copy link
Contributor

I've tweaked the code to keep a map of all the files and their last modified times, basing namespace file changes on that instead of the current system time. My initial pass did a double pass on all the files for every check, but this version only does a single pass, so besides the memory overhead of the map, it's effectively the same I/O cost.

@CmdrDats
Copy link
Contributor Author

Hmm, I'm noticing that this is failing tests.. I'll look into it..

… files, initial dependency graph was incorrectly setup and "src" was hardcoded.
@weavejester
Copy link
Owner

Could you fix the commit messages so that the first line is less than 70 characters? In general, Git commit messages have a short first line, and then a more detailed description on later lines. That way you don't get a "cut off" message when viewing the logs in GitHub or with git log

(defn- newer-sources [dirs timestamp]
(filter #(> (.lastModified %) timestamp) (find-sources dirs)))
(defn- current-timestamp-map
"Get a the current modified timestamp map for all sources"
Copy link
Owner

Choose a reason for hiding this comment

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

Small typo

Instead of comparing all file modification dates against a fixed time,
a map is kept to determine which have changed. Fixes #3.
Fixed broken tests, last modified file needed to handle new files, initial dependency graph was incorrectly setup and "src" was hardcoded.
@CmdrDats
Copy link
Contributor Author

CmdrDats commented Oct 9, 2012

Ok, I've amended the commit - I hope that was the correct procedure :/ I'll be careful not to have too long commit messages in future - also fixed the small typo :)

@weavejester
Copy link
Owner

Could you rebase the changes so the history is clean? i.e. perform a git rebase to get rid of the merge commit and to join the other commits into one.

@CmdrDats
Copy link
Contributor Author

CmdrDats commented Oct 9, 2012

I tried that locally, and it just ended up changing the log back to the long 70+ character log. I guess I can delete my fork again and reapply the changes.. just seems a bit overkill for such a simple change.

@weavejester
Copy link
Owner

The trick is to perform an interactive rebase against my Hiccup repository:

$ git remote add upstream https://github.com/weavejester/ns-tracker.git
$ git fetch upstream
$ git rebase -i upstream/master

This will get you some text in an editor like this:

pick 6e9958b Issue 3 - Monitor against individual file timestamp changes instead of System/currentTimeMillis
pick 7a4be48 Issue 3 - Broken tests and "src" was hardcoded
pick f9b7dfc Issue 3 - Fixed broken tests, last modified file needed to handle new files, initial dependency graph was incorrectly setup and "src" was hardcoded.
pick a5f5af9 Fix small typo in current-timestamp-map

What we want to do is to squash all of the commits together, so:

pick 6e9958b Issue 3 - Monitor against individual file timestamp changes instead of System/currentTimeMillis
squash 7a4be48 Issue 3 - Broken tests and "src" was hardcoded
squash f9b7dfc Issue 3 - Fixed broken tests, last modified file needed to handle new files, initial dependency graph was incorrectly setup and "src" was hardcoded.
squash a5f5af9 Fix small typo in current-timestamp-map

Save and quit, then you'll get another editor for writing the new commit message. You could write something like:

Monitor against individual file timestamp changes

Instead of comparing all file modification dates against a fixed time,
a map is kept to determine which have changed. Fixes #3.

Save and quit again, and your history should be clean and tidy.

@CmdrDats
Copy link
Contributor Author

CmdrDats commented Oct 9, 2012

Thanks, that was a helpful and interesting, but i did that and ended up with yet another merge message.

So, I've deleted that fork and created a new one, really can't afford to be messing around with git repo's, I have actual work to get done.

@CmdrDats CmdrDats closed this Oct 9, 2012
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