WIP config: allow user to know type of config options #2399

ROGERSM94 · 2019-11-08T22:04:07Z

This is just a proof of concept for the change mentioned in #2348 by @PhilipOakley ... I just want to make sure I have the general idea here right.

Expands the config --show-origin option to print out the kind of config
file that the config setting originates from (local/system/global/etc.).
This is just proof of concept code that will eventually be moved to its
own option.

TODO: create an option for this instead of adding to output of
show-origin.

dscho · 2019-11-09T10:27:20Z

I allowed myself to edit the PR description, deleting the irrelevant parts on the bottom. Hope you don't mind.

dscho · 2019-11-09T10:28:29Z

builtin/config.c

+static void show_config_origin_type(struct strbuf *buf)
+{
+	const char *scope;
+	const char term = end_null ? '\0' : '\t';


It's NUL, not NULL... But that's not a problem you introduced.

dscho · 2019-11-09T10:30:02Z

builtin/config.c

 		show_config_origin(buf);
+		show_config_origin_type(buf);


I am not quite sure that we can change the output of git config, in particular with -z (which is intended for scripts or other applications to consume, and which would be broken by this change)...

dscho · 2019-11-09T10:34:12Z

Thank you for working on this!

As I mentioned in a comment on the patch, I don't think that we can change the output format as liberally, this would have to be a new mode. Maybe we can teach --show-origin to take an optional argument?

I imagine that --show-origin=pretty would actually output --system instead of C:/Program Files/Git/etc/gitconfig, and if the actual file was included, output something like C:/Users/rogersm94/all-config (included from --system).

What do you think?

dscho · 2019-11-09T10:35:50Z

builtin/config.c

+		strbuf_addstr(buf, scope);
+	else
+		quote_c_style(scope, buf, NULL, 0);
+	strbuf_addch(buf, term);


A slightly cleaner approach, I think, would be to change show_config_origin_type() to return the string instead, probably renaming it to origin_type_to_string(), so that it has only one concern, and is independent of the concern to write the output as part of show_all_config().

Normally I'd agree with you, I was just trying to match the style of show_config_origin(), but besides that there's no reason to not just return the string.

Fair enough. I'd still contend that this is not a style issue, but that show_config_origin() is an example I'd rather not follow.

ROGERSM94 · 2019-11-09T12:29:11Z

I definitely like the idea of not having this be the default output, so I'll have that change up soon.

ROGERSM94 · 2019-11-09T23:37:50Z

I've pushed some changes with the discussion in mind. I haven't had time to update the tests yet. But I just added this as its own option since that wasn't too difficult as (--show-scope) since this is kind of a different question than "what file did this come from".

dscho · 2019-11-10T19:55:58Z

I just added this as its own option since that wasn't too difficult as (--show-scope) since this is kind of a different question than "what file did this come from".

Ah, but I did mean it as "what file did this come from" mode. That is, in show_config_origin(), guarded by a new "pretty" mode, I would want to output the appropriate option if current_config_name() corresponds to one of the three configs (skipping the xdg config because there is no corresponding git config option).

I.e. if current_config_name() equals git_etc_gitconfig(), I would output --system, if it matches expand_user_path("~/.gitconfig", 0), I would output --global, if it is equal to git_path("config"), I would output --repo. (Possibly caching the check in some manner...)

ROGERSM94 · 2019-11-10T21:43:50Z

The problem I have with doing it that way, is that we're making doing this calculation to figure out what the "scope" of the config variable is without using the actual scope value... Would it make more sense to just have a different option output a list of files that the config uses and which ones correspond to the global/system/etc. ? I'd think that that makes more sense if the goal is to resolve confusion about what files correspond to which scope

dscho · 2019-11-11T12:36:05Z

Well, I thought it'd be neat for git config --show-origin=pretty -l -f "C:/Program Files/Git/etc/gitconfig" to show --system. Very user-friendly in my mind...

ROGERSM94 · 2019-11-11T16:53:11Z

So the --show-origin=pretty vs. different option thing makes sense, so i'll move to that.

I'm more concerned with the method for determining it. Upon further reading in the documentation, it seems that the files commonly used for git config searches are well documented (i.e. , making me loathe to do something like you suggested:

I.e. if current_config_name() equals git_etc_gitconfig(), I would output --system, if it matches expand_user_path("~/.gitconfig", 0), I would output --global, if it is equal to git_path("config"), I would output --repo. (Possibly caching the check in some manner...)

So I definitely think the better approach is to use the scope that the software keeps track of. This could get confusing though in the case of the same file being in multiple config files i.e. having an [includeIf] directive in your global config and an [includeIf] and local config, but both including the same file at different times would cause the output to display a different scope depending on the conditionals. Although I don't think this is really the common use case. I haven't really
looked into how includes are handled (i'm currently looking into that). But I don't really know what the ideal way to handle this case would be.

PhilipOakley · 2019-11-11T23:21:50Z

Personally I'd be quite happy with an independent option --show-type (or --show-scope as above) as per #2348 (comment).

I'm not sure whether the coding and possible upstream discussions for the =pretty is worth it.

The one thing I haven't checked, as my configs don't include them, is the [Include] configs, though I expect from a users viewpoint they'll want to see which of the three config scopes it arrived from. I.e. the investigation is from the config to the scope to the file, not the other way around where the -f file is given

ROGERSM94 · 2019-11-13T00:04:58Z

So I did some investigation and the config scope that's calculated respects includes. Although there may be a bug in that using files/blobs doesn't parse as any of the known configs... I feel as though they should be scoped as command line. From my investigation it seems that it's because the --file and --blob options are done in the builtin\config.c file, and they just call the parsing functions used in the \config.c file which are independent of the sequence where that scoping is actually set. I'm not really sure on the best way to resolve that issue, maybe explicitly check if it's from a --file or --blob and then set it to display that it's from command line?

ROGERSM94 · 2019-11-15T01:23:00Z

I still haven't really updated the tests, but on further thought I like the idea of --show-scope being a separate command line option like --show-origin as it can be displayed independently of the actual origin. I also made the decision in this example that --file, --blob, etc. are to be considered "command line" in scope same as config options given via -c... I didn't really have a better name for them to be honest

Instead of rolling our own functionality to test the number of lines a command outputs, use test_line_count() which provides better debugging information in the case of a failure. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

When wrapping a git command in a command substitution within another command, we throw away the git command's exit code. In case the git command fails, we would like to know about it rather than the failure being silent. Extract git commands so that their exit codes are not lost. Instead of using `test -n` or `test -z`, replace them respectively with invocations of test_file_not_empty() and test_must_be_empty() so that we get better debugging information in the case of a failure. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

In case an invocation of `git rev-list` fails within the command substitution, the failure will be masked. Remove the command substitution and use test_cmp_rev() so that failures can be discovered. This change was done with the following sed expressions: s/test "$(git rev-parse.* $[^)]*$)" = "$(git rev-parse $[^)]*$)"/test_cmp_rev \1 \2/ s/test $[^ ]*$ = "$(git rev-parse.* $[^)]*$)"/test_cmp_rev \1 \2/ s/test "$(git rev-parse.* $[^)]*$)" != "$(git rev-parse.* $[^)]*$)"/test_cmp_rev ! \1 \2/ s/test $[^ ]*$ != "$(git rev-parse.* $[^)]*$)"/test_cmp_rev ! \1 \2/ Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

In case an invocation of a git command fails within the command substitution, the failure will be masked. Replace the command substitution with a file-redirection and a call to test_cmp. This change was done with the following GNU sed expressions: s/$\s*$test $[^ ]*$ = "$($git [^)]*$)"/\1echo \2 >expect \&\&\n\1\3 >actual \&\&\n\1test_cmp expect actual/ s/$\s*$test "$($git [^)]*$)" = $[^ ]*$/\1echo \3 >expect \&\&\n\1\2 >actual \&\&\n\1test_cmp expect actual/ A future patch will clean up situations where we have multiple duplicate statements within a test case. This is done to keep this patch purely mechanical. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Before, if the invocation of git failed, it would be masked by the pipe since only the return code of the last element of a pipe is used. Rewrite the test to put the git command on its own line so its return code is not masked. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

We currently have many instances of `test <line> = $(cat <file>)` and `test $(cat <file>) = <line>`. In the case where this fails, it will be difficult for a developer to debug since the output will be masked. Replace these instances with invocations of test_cmp(). This change was done with the following GNU sed expressions: s/$\s*$test $[^=]*$= "$(cat $[^)]*$)"/\1echo \2>expect \&\&\n\1test_cmp expect \3/ s/$\s*$test "$(cat $[^)]*$)" = $[^&]*$$ &&$\?$/\1echo \3 >expect \&\&\n\1test_cmp expect \2\4/ A future patch will clean up situations where we have multiple duplicate statements within a test case. This is done to keep this patch purely mechanical. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

In the previous patches, the mechanical application of changes left some duplicate statements in the test case which were not strictly incorrect but were redundant and possibly misleading. Remove these duplicate statements so that it is clear that the intent behind the tests are that the content of the file stays the same throughout the whole test case. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Currently, if a git command fails in an unexpected way, such as a segfault, it will be masked and ignored. Replace the ! with test_must_fail so that only expected failures pass. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

The code style for tests is to have statements on their own line if possible. Move the `then` onto its own line so that it conforms with the test style. Signed-off-by: Denton Liu <liu.denton@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

The index might be aware that a file hasn't modified via fsmonitor, but unpack-trees did not pay attention to it and checked via ie_match_stat which can be inefficient on certain filesystems. This significantly slows down commands that run oneway_merge, like checkout and reset --hard. This patch makes oneway_merge check whether a file is considered unchanged through fsmonitor and skips ie_match_stat on it. unpack-trees also now correctly copies over fsmonitor validity state from the source index. Finally, for correctness, we force a refresh of fsmonitor state in tweak_fsmonitor. After this change, commands like stash (that use reset --hard internally) go from 8s or more to ~2s on a 250k file repository on a mac. Helped-by: Junio C Hamano <gitster@pobox.com> Helped-by: Kevin Willford <Kevin.Willford@microsoft.com> Signed-off-by: Utsav Shah <utsav@dropbox.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Introduce a static replace_cstring() function to simplify repeated pattern of free-and-xmemdupz() for GPG status line parsing. This also helps us avoid potential memleaks if parsing of new status lines are introduced in the future. Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

The VALIDSIG status line from GnuPG with --status-fd is documented to have 9 required and 1 optional fields [1]. The final, and optional, field is used to specify the fingerprint of the primary key that made the signature in case it was made by a subkey. However, this field is only available for OpenPGP signatures; not for CMS/X.509. If the VALIDSIG status line does not have the optional 10th field, the current code will continue reading onto the next status line. And this is the case for non-OpenPGP signatures [1]. The consequence is that a subsequent status line may be considered as the "primary key" for signatures that does not have an actual primary key. Limit the search of these 9 or 10 fields to the single line to avoid this problem. If the 10th field is missing, report that there is no primary key fingerprint. [Reference] [1] GnuPG Details, General status codes https://git.gnupg.org/cgi-bin/gitweb.cgi?p=gnupg.git;a=blob;f=doc/DETAILS;h=6ce340e8c04794add995e84308bb3091450bd28f;hb=HEAD#l483 The documentation says: VALIDSIG <args> The args are: - <fingerprint_in_hex> - <sig_creation_date> - <sig-timestamp> - <expire-timestamp> - <sig-version> - <reserved> - <pubkey-algo> - <hash-algo> - <sig-class> - [ <primary-key-fpr> ] This status indicates that the signature is cryptographically valid. [...] PRIMARY-KEY-FPR is the fingerprint of the primary key or identical to the first argument. The primary-key-fpr parameter is used for OpenPGP and not available for CMS signatures. [...] Signed-off-by: Hans Jerry Illikainen <hji@dyntopia.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Reduce unnecessary reading of state variables back from the disk during sequencer operation. * ag/sequencer-todo-updates: sequencer: directly call pick_commits() from complete_action() rebase: fill `squash_onto' in get_replay_opts() sequencer: move the code writing total_nr on the disk to a new function sequencer: update `done_nr' when skipping commands in a todo list sequencer: update `total_nr' when adding an item to a todo list

Avoid gmtime() and localtime() and prefer their reentrant counterparts. * dd/time-reentrancy: mingw: use {gm,local}time_s as backend for {gm,local}time_r archive-zip.c: switch to reentrant localtime_r date.c: switch to reentrant {gm,local}time_r

"git add -i" that is getting rewritten in C has been extended to cover subcommands other than the "patch". * js/builtin-add-i-cmds: built-in add -i: offer the `quit` command built-in add -i: re-implement the `diff` command built-in add -i: implement the `patch` command built-in add -i: re-implement `add-untracked` in C built-in add -i: re-implement `revert` in C built-in add -i: implement the `update` command built-in add -i: prepare for multi-selection commands built-in add -i: allow filtering the modified files list add-interactive: make sure to release `rev.prune_data`

In a repository with many packfiles, the cost of the procedure that avoids registering the same packfile twice was unnecessarily high by using an inefficient search algorithm, which has been corrected. * cs/store-packfiles-in-hashmap: packfile.c: speed up loading lots of packfiles

Test cleanup. * dl/test-cleanup: (26 commits) t7700: stop losing return codes of git commands t7700: make references to SHA-1 generic t7700: replace egrep with grep t7700: consolidate code into test_has_duplicate_object() t7700: consolidate code into test_no_missing_in_packs() t7700: s/test -f/test_path_is_file/ t7700: move keywords onto their own line t7700: remove spaces after redirect operators t7700: drop redirections to /dev/null t7501: stop losing return codes of git commands t7501: remove spaces after redirect operators t5703: stop losing return codes of git commands t5703: simplify one-time-sed generation logic t5317: use ! grep to check for no matching lines t5317: stop losing return codes of git commands t4138: stop losing return codes of git commands t4015: use test_write_lines() t4015: stop losing return codes of git commands t3600: comment on inducing SIGPIPE in `git rm` t3600: stop losing return codes of git commands ...

"git rebase" did not work well when format.useAutoBase configuration variable is set, which has been corrected. * dl/rebase-with-autobase: rebase: fix format.useAutoBase breakage format-patch: teach --no-base t4014: use test_config() format-patch: fix indentation t3400: demonstrate failure with format.useAutoBase

The "diff" machinery learned not to lose added/removed blank lines in the context when --ignore-blank-lines and --function-context are used at the same time. * rs/xdiff-ignore-ws-w-func-context: xdiff: unignore changes in function context

* hw/doc-in-header: trace2: move doc to trace2.h submodule-config: move doc to submodule-config.h tree-walk: move doc to tree-walk.h trace: move doc to trace.h run-command: move doc to run-command.h parse-options: add link to doc file in parse-options.h credential: move doc to credential.h argv-array: move doc to argv-array.h cache: move doc to cache.h sigchain: move doc to sigchain.h pathspec: move doc to pathspec.h revision: move doc to revision.h attr: move doc to attr.h refs: move doc to refs.h remote: move doc to remote.h and refspec.h sha1-array: move doc to sha1-array.h merge: move doc to ll-merge.h graph: move doc to graph.h and graph.c dir: move doc to dir.h diff: move doc to diff.h and diffcore.h

Code clean-up. * dl/range-diff-with-notes: range-diff: clear `other_arg` at end of function range-diff: mark pointers as const t3206: fix incorrect test name

Test coverage update in preparation for further work on "git add -i". * js/add-i-a-bit-more-tests: apply --allow-overlap: fix a corner case git add -p: use non-zero exit code when the diff generation failed t3701: verify that the diff.algorithm config setting is handled t3701: verify the shown messages when nothing can be added t3701: add a test for the different `add -p` prompts t3701: avoid depending on the TTY prerequisite t3701: add a test for advanced split-hunk editing

The test on "fast-import" used to get stuck when "fast-import" died in the middle. * sg/t9300-robustify: t9300-fast-import: don't hang if background fast-import exits too early t9300-fast-import: store the PID in a variable instead of pidfile

Message fix. * dr/branch-usage-casefix: l10n: minor case fix in 'git branch' '--unset-upstream' description

Test cleanup. * js/t3404-indent-fix: t3404: fix indentation

Code cleanup. * rs/archive-zip-code-cleanup: archive-zip: use enum for compression method

Code cleanup. * rs/commit-export-env-simplify: commit: use strbuf_add() to add a length-limited string

Code cleanup. * rs/patch-id-use-oid-to-hex: patch-id: use oid_to_hex() to print multiple object IDs

Signed-off-by: Junio C Hamano <gitster@pobox.com>

t3434.3 was fixed by commit 917d0d6 ("Merge branch 'js/rebase-r-safer-label'", 2019-12-05). t3434 did not exist in js/rebase-r-safer-label, so could not have marked the test as fixed, and it was probably not noticed that the merge fixed this test. Mark it as fixed now. Signed-off-by: Elijah Newren <newren@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

In git config use of the end_null variable to determine if we should be null terminating our output. While it is correct to say a string is "null terminated" the character is actually the "nul" character, so this malapropism is being fixed. Signed-off-by: Matthew Rogers <mattr94@gmail.com>

Previously when iterating through git config variables, worktree config and local config were both considered "CONFIG_SCOPE_REPO". This was never a problem before as no one had needed to differentiate between the two cases. Additionally we rename what was CONFIG_SCOPE_REPO to CONFIG_SCOPE_LOCAL to reflect its new, more specific meaning. The clients of 'current_config_scope()' who cared about CONFIG_SCOPE_REPO are also modified to similarly care about CONFIG_SCOPE_WORKTREE and CONFIG_SCOPE_LOCAL to preserve previous behavior. Signed-off-by: Matthew Rogers <mattr94@gmail.com>

When a user queries config values with --show-origin, often it's difficult to determine what the actual "scope" (local, global, etc.) of a given value is based on just the origin file. Teach 'git config' the '--show-scope' option to print the scope of all displayed config values. Signed-off-by: Matthew Rogers <mattr94@gmail.com>

dscho · 2019-12-25T11:32:30Z

Seeing as this branch conflicts with Git for Windows' master because it targets upstream Git's master, maybe close this here PR, at least for now?

Or is your plan to rebase the branch once it makes it into Git's master so that we can merge it into Git for Windows' master before the next Git release?

ROGERSM94 · 2019-12-25T13:14:14Z

I can close it, I just wasn't sure If I should

dscho · 2019-12-25T20:29:43Z

Thanks!

ROGERSM94 mentioned this pull request Nov 8, 2019

Lacking a name for the fourth G-f-W config file #2348

Closed

1 task

dscho reviewed Nov 9, 2019

View reviewed changes

ROGERSM94 force-pushed the add-config-flags branch from 7ae3ba5 to 6ef41d5 Compare November 9, 2019 23:32

ROGERSM94 force-pushed the add-config-flags branch from 6ef41d5 to 0fff2c7 Compare November 15, 2019 01:15

weekly-digest bot mentioned this pull request Nov 17, 2019

Weekly Digest (10 November, 2019 - 17 November, 2019) #2408

Closed

Denton-L and others added 12 commits November 21, 2019 09:41

gitster added 17 commits December 16, 2019 13:08

Merge branch 'dd/time-reentrancy'

4755a34

Avoid gmtime() and localtime() and prefer their reentrant counterparts. * dd/time-reentrancy: mingw: use {gm,local}time_s as backend for {gm,local}time_r archive-zip.c: switch to reentrant localtime_r date.c: switch to reentrant {gm,local}time_r

Merge branch 'dl/range-diff-with-notes'

d1c0fe8

Code clean-up. * dl/range-diff-with-notes: range-diff: clear `other_arg` at end of function range-diff: mark pointers as const t3206: fix incorrect test name

Merge branch 'sg/t9300-robustify'

8bc481f

The test on "fast-import" used to get stuck when "fast-import" died in the middle. * sg/t9300-robustify: t9300-fast-import: don't hang if background fast-import exits too early t9300-fast-import: store the PID in a variable instead of pidfile

Merge branch 'dr/branch-usage-casefix'

3a44db2

Message fix. * dr/branch-usage-casefix: l10n: minor case fix in 'git branch' '--unset-upstream' description

Merge branch 'js/t3404-indent-fix'

4438a1a

Test cleanup. * js/t3404-indent-fix: t3404: fix indentation

Merge branch 'rs/archive-zip-code-cleanup'

43bf44e

Code cleanup. * rs/archive-zip-code-cleanup: archive-zip: use enum for compression method

Merge branch 'rs/commit-export-env-simplify'

e3b7239

Code cleanup. * rs/commit-export-env-simplify: commit: use strbuf_add() to add a length-limited string

Merge branch 'rs/patch-id-use-oid-to-hex'

59d0b3b

Code cleanup. * rs/patch-id-use-oid-to-hex: patch-id: use oid_to_hex() to print multiple object IDs

The sixth batch

b02fd2a

Signed-off-by: Junio C Hamano <gitster@pobox.com>

ROGERSM94 force-pushed the add-config-flags branch from 6f01233 to ec699bb Compare December 17, 2019 00:20

weekly-digest bot mentioned this pull request Dec 22, 2019

Weekly Digest (15 December, 2019 - 22 December, 2019) #2441

Closed

ROGERSM94 force-pushed the add-config-flags branch from ec699bb to 0ed8157 Compare December 22, 2019 17:38

ROGERSM94 added 2 commits December 22, 2019 12:54

ROGERSM94 force-pushed the add-config-flags branch from 0ed8157 to e5a8fe9 Compare December 22, 2019 17:55

ROGERSM94 force-pushed the add-config-flags branch from e5a8fe9 to 066d944 Compare December 24, 2019 02:35

ROGERSM94 closed this Dec 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP config: allow user to know type of config options #2399

WIP config: allow user to know type of config options #2399

ROGERSM94 commented Nov 8, 2019 •

edited by dscho

Loading

dscho commented Nov 9, 2019

dscho Nov 9, 2019

dscho Nov 9, 2019

dscho commented Nov 9, 2019

dscho Nov 9, 2019

ROGERSM94 Nov 9, 2019

dscho Nov 10, 2019

ROGERSM94 commented Nov 9, 2019

ROGERSM94 commented Nov 9, 2019

dscho commented Nov 10, 2019 •

edited

Loading

ROGERSM94 commented Nov 10, 2019

dscho commented Nov 11, 2019

ROGERSM94 commented Nov 11, 2019

PhilipOakley commented Nov 11, 2019

ROGERSM94 commented Nov 13, 2019 •

edited

Loading

ROGERSM94 commented Nov 15, 2019

dscho commented Dec 25, 2019

ROGERSM94 commented Dec 25, 2019

dscho commented Dec 25, 2019

WIP config: allow user to know type of config options #2399

WIP config: allow user to know type of config options #2399

Conversation

ROGERSM94 commented Nov 8, 2019 • edited by dscho Loading

dscho commented Nov 9, 2019

dscho Nov 9, 2019

Choose a reason for hiding this comment

dscho Nov 9, 2019

Choose a reason for hiding this comment

dscho commented Nov 9, 2019

dscho Nov 9, 2019

Choose a reason for hiding this comment

ROGERSM94 Nov 9, 2019

Choose a reason for hiding this comment

dscho Nov 10, 2019

Choose a reason for hiding this comment

ROGERSM94 commented Nov 9, 2019

ROGERSM94 commented Nov 9, 2019

dscho commented Nov 10, 2019 • edited Loading

ROGERSM94 commented Nov 10, 2019

dscho commented Nov 11, 2019

ROGERSM94 commented Nov 11, 2019

PhilipOakley commented Nov 11, 2019

ROGERSM94 commented Nov 13, 2019 • edited Loading

ROGERSM94 commented Nov 15, 2019

dscho commented Dec 25, 2019

ROGERSM94 commented Dec 25, 2019

dscho commented Dec 25, 2019

ROGERSM94 commented Nov 8, 2019 •

edited by dscho

Loading

dscho commented Nov 10, 2019 •

edited

Loading

ROGERSM94 commented Nov 13, 2019 •

edited

Loading