[#200] Warnings about files that weren't added to git yet #215

Sorokin-Anton · 2022-11-02T17:53:03Z

Description

Problem: after 0.2.2 release, xrefcheck cares only about files that were added to Git. That can be confusing for users (see #200)

Solution:
If a scannable (currently it means markdown) file is not ignored (by git or via config) and not tracked by git, print a warning to stderr while scanning repo.

If a link target such file, change error message from "file not exists" to Link target is not tracked by Git

Suggest user to run "git add" before running xrefcheck in both cases.

To do this, I've changed the RepoInfo type, so it also contains information about untracked files now.

Also add CLI option 1--include-untracked` to treat such files as existing.

Related issue(s)

Fixes #200

✅ Checklist for your Pull Request

Ideally a PR has all of the checkmarks set.

If something in this list is irrelevant to your PR, you should still set this
checkmark indicating that you are sure it is dealt with (be that by irrelevance).

Related changes (conditional)

Tests
- If I added new functionality, I added tests covering it.
- If I fixed a bug, I added a regression test to prevent the bug from
  silently reappearing again.
Documentation
- I checked whether I should update the docs and did so if necessary:
  - README
  - Haddock
Public contracts
- Any modifications of public contracts comply with the Evolution
  of Public Contracts policy.
- I added an entry to the changelog if my changes are visible to the users
  and
- provided a migration guide for breaking changes if possible

Stylistic guide (mandatory)

My commits comply with the policy used in Serokell.
My code complies with the style guide.

Martoon-00

Thanks, looks very good to me 👍

I would like to take another look after rebase.

Martoon-00 · 2022-11-09T22:33:32Z

src/Xrefcheck/Scan.hs

+    location =
+      if root `equalFilePath` "."
+        then ""
+        else dropTrailingPathSeparator root


After all, getUntrackedFiled half-duplicates readDirectoryWith, right?

Removed code duplication in last commit

In this regard, do you plan to retain the last commit or squash it afterward?

I like how it is put separately, but it contains a lot of changes to the first commit and this is not good.

I was going to retain it, but now see problem you mentioned, so I will better move some changes between commits during interactive rebase (I'll write here when done)

Cool, will wait for your message.

@Martoon-00 done

Thank you, looks good 👍

Martoon-00 · 2022-11-09T22:38:28Z

Also, as we discussed in Slack, let's add an option (probably a command line option?) to treat untracked git files as already added. FTR, my use case is sanity-checking auto-generated markdown documentation which we have in Morley.

Sorokin-Anton · 2022-11-10T22:05:14Z

Also, as we discussed in Slack, let's add an option (probably a command line option?) to treat untracked git files as already added. FTR, my use case is sanity-checking auto-generated markdown documentation which we have in Morley.

Done, also rebased on master

Martoon-00

Looks very good to me 👍

I have mostly cosmetic concerns, and likely that will be it.

src/Xrefcheck/Core.hs

src/Xrefcheck/CLI.hs

Martoon-00 · 2022-11-13T20:02:34Z

Oh and just in case (I wanted to mention this, but didn't expect you to read my messages close to Sunday's midnight 😅): this all is absolutely fine to fix some later (Monday/Tuesday), no hurry is needed. Unless you have a wish and opportinity to work on it right now 🙂

Sorokin-Anton · 2022-11-13T20:08:33Z

Of course, I am just checking github notifications to understand choices of tasks for tomorrow

Martoon-00 · 2022-11-16T20:17:29Z

In the second commit, you seem to have no subject, it immediately starts from the description. Probably this accidentally happened during rebase?

Aside from that, LGTM.

Martoon-00

One last nitpick from my side remains, but I preliminarily approve, feel free to merge after fixing that.

Problem: after 0.2.2 release, xrefcheck cares only about files that were added to Git. That can be confusing for users (see #200) Solution: If a scannable (currently it means markdown) file is not ignored (by git or via config) and not tracked by git, print a warning to stderr while scanning repo. If a link target such file, change error message from "file not exists" to `Link target is not tracked by Git` Suggest user to run "git add" before running xrefcheck in both cases. To do this, I've changed the `RepoInfo` type, so it also contains information about untracked files now.

Problem: xrefcheck checks only files that are tracked by Git, but sometimes we want to run xrefcheck on files without adding them to Git, e.g. when we want to test some generator of markdown files or when we actively create markdown files during development. Solution: add option to treat files that were neither added to git nor ignored as existing.

Sorokin-Anton requested review from dcastro and Sereja313 November 2, 2022 17:53

Sorokin-Anton force-pushed the Sorokin-Anton/#208-remove-exessive-newlines branch 2 times, most recently from 860c847 to 1476bd3 Compare November 4, 2022 12:27

dcastro requested a review from Martoon-00 November 4, 2022 15:20

Base automatically changed from Sorokin-Anton/#208-remove-exessive-newlines to master November 4, 2022 15:41

Martoon-00 reviewed Nov 9, 2022

View reviewed changes

Sorokin-Anton force-pushed the Sorokin-Anton/#200-untracked-files-hints branch from e46f4a8 to 6ad13ae Compare November 10, 2022 14:55

Sorokin-Anton requested a review from Martoon-00 November 10, 2022 22:05

Martoon-00 reviewed Nov 13, 2022

View reviewed changes

src/Xrefcheck/Core.hs Outdated Show resolved Hide resolved

Martoon-00 reviewed Nov 13, 2022

View reviewed changes

src/Xrefcheck/CLI.hs Show resolved Hide resolved

Sorokin-Anton force-pushed the Sorokin-Anton/#200-untracked-files-hints branch from 02967fe to 24c3bd9 Compare November 15, 2022 22:53

Martoon-00 approved these changes Nov 16, 2022

View reviewed changes

Sorokin-Anton added 2 commits November 17, 2022 15:32

Sorokin-Anton force-pushed the Sorokin-Anton/#200-untracked-files-hints branch from 24c3bd9 to 1c0fbfe Compare November 17, 2022 13:38

Sorokin-Anton merged commit 68a9098 into master Nov 17, 2022

Sorokin-Anton deleted the Sorokin-Anton/#200-untracked-files-hints branch December 1, 2022 12:01

int-index mentioned this pull request Dec 27, 2024

v0.3.0 release #310

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[#200] Warnings about files that weren't added to git yet #215

[#200] Warnings about files that weren't added to git yet #215

Sorokin-Anton commented Nov 2, 2022 •

edited

Loading

Martoon-00 left a comment

Martoon-00 Nov 9, 2022

Sorokin-Anton Nov 10, 2022

Martoon-00 Nov 11, 2022

Sorokin-Anton Nov 11, 2022 •

edited

Loading

Martoon-00 Nov 13, 2022

Sorokin-Anton Nov 15, 2022

Martoon-00 Nov 16, 2022

Martoon-00 commented Nov 9, 2022

Sorokin-Anton commented Nov 10, 2022 •

edited

Loading

Martoon-00 left a comment

Martoon-00 commented Nov 13, 2022

Sorokin-Anton commented Nov 13, 2022

Martoon-00 commented Nov 16, 2022

Martoon-00 left a comment

[#200] Warnings about files that weren't added to git yet #215

[#200] Warnings about files that weren't added to git yet #215

Conversation

Sorokin-Anton commented Nov 2, 2022 • edited Loading

Description

Related issue(s)

✅ Checklist for your Pull Request

Related changes (conditional)

Stylistic guide (mandatory)

Martoon-00 left a comment

Choose a reason for hiding this comment

Martoon-00 Nov 9, 2022

Choose a reason for hiding this comment

Sorokin-Anton Nov 10, 2022

Choose a reason for hiding this comment

Martoon-00 Nov 11, 2022

Choose a reason for hiding this comment

Sorokin-Anton Nov 11, 2022 • edited Loading

Choose a reason for hiding this comment

Martoon-00 Nov 13, 2022

Choose a reason for hiding this comment

Sorokin-Anton Nov 15, 2022

Choose a reason for hiding this comment

Martoon-00 Nov 16, 2022

Choose a reason for hiding this comment

Martoon-00 commented Nov 9, 2022

Sorokin-Anton commented Nov 10, 2022 • edited Loading

Martoon-00 left a comment

Choose a reason for hiding this comment

Martoon-00 commented Nov 13, 2022

Sorokin-Anton commented Nov 13, 2022

Martoon-00 commented Nov 16, 2022

Martoon-00 left a comment

Choose a reason for hiding this comment

Sorokin-Anton commented Nov 2, 2022 •

edited

Loading

Sorokin-Anton Nov 11, 2022 •

edited

Loading

Sorokin-Anton commented Nov 10, 2022 •

edited

Loading