Don't touch files if `--no-reformat` is passed

[robertberrington]

After manually adding an item, I can edit the text. The reviewed value is null since I haven't made an initial pass in doorstop where reviewed is initially trusted. (doorstop will calculate the hash, then add the file to the VCS).

This leaves repo in a "dirty" state, but there are no warnings/errors to identify this.

In my Makefile I run these basic commands:

1. Check the repo is clean (or exit -1 if dirty)
2. Run $ doorstop --error-all to verify static analysis
3. [do important stuff like generate zip files of outputs]

There is an opportunity for $ doorstop --error-all to find reviewed and links that can be trusted ... changing the files and having a dirty repo. But if I don't re-check for changes, the system will move forward and [do stuff].

In fact, simply adding a dirty check does NOT fail if the files are added, but not committed.

Dirty check is performed by: $ git diff --name-only --exit-code ... Rats! This doesn't work!!! This doesn't error if the file is added but not committed.

Do this instead: $ git diff-index --name-status --exit-code HEAD ... This will catch uncommitted changes to tracked files AND files added but not committed.

[jacebrowning]

This doesn't seem to have much to do with this project, but I think this is what you're looking for: http://stackoverflow.com/a/3879077/429533

[robertberrington]

Should (or can) Doorstop generate a WARNING if it automatically trusting new items and links? Currently there is no output. After each run of doorstop, I have to manually check with $ git status.

[jacebrowning]

Doesn't fixing your "dirty" check to include uncommitted staged changes resolve this issue?

[robertberrington]

It can be solved that way. However I have a little hang-up that I think this issue best exemplifies.

There are two basic operations in doorstop:

1. Operations to change the data
2. Operations to validate/test the data

I think it is really neat that simply executing $ doorstop at the command line runs a default validation. However, it also has the power to change the data.

I wonder if these two should be separated.

Is it true that if I add/commit changes that don't automatically trust new items/links that my CI could run $ doorstop on the server side and end up with different data than what is in the repo?

I can add the dirty check, but does it cross the threshold of "not intuitive".

[jacebrowning]

As long as you are running doorstop before you commit, there shouldn't be any new changes appearing on other machines.

Adding the --no-reformat option should prevent files from being modified if you ONLY care about the return code.

[robertberrington]

Hmm... I don't think that option does what you think it does ...

I manually reverted item.reviewed to null and committed it to VCS.

$ doorstop --no-level-check --no-reformat -v
...
...
checking item CM-TPS-0024...
marking item as reviewed...
...

At the end of the process, CM-TPS-0024 item.reviewed was automatically updated.

I would have expected the --no-reformat option to:

1. NOT modify items
2. Generate WARNING's for those operations being suppressed

[jacebrowning]

You will also need to pass a --no-review-check there to leave the review status alone.

I somewhat understand your concern that running doorstop can changes files (review/link status), but I think if your workflow is:

1. make changes
2. run doorstop
3. commit

you shouldn't have any issues.

[robertberrington]

Running $ doorstop --no-level-check --no-reformat --no-review-check does not modify the requirement, but it also does NOT produce a WARNING of a missing initial review.

The point of my check is to tell me is something "important" is missing from the requirements so that I know I'm safe to commit/publish them.

So, what is the point of the --no-reformat option if running it modifies the reviewed property?

$ doorstop -h
...
  -F, --no-reformat        do not reformat item files during validation
...

As stated above:

I would have expected the --no-reformat option to:

1. NOT modify items
2. Generate WARNING's for those operations being suppressed

What do you think about a --no-trust-new option where reviewed = null and links: [item.uid] = null would NOT be modified, but a WARNING would be displayed that can return stderr for make?

Thanks for working through this use case with me! I have a fix that works around what I want to do in my Makefile, but I wasn't sure if there was a broader scope to which this could apply.

[jacebrowning]

I'll consider this a feature request to prevent ALL file modification if --no-reformat is passed:

• don't add the initial review stamp
• don't add the initial links stamps

Potentially this flag should be renamed to --no-update.