> > +
>
> Addition of trailing whitespace?
>
> You can avoid such mistakes in the future by enabling our sample
> pre-commit hook, which essentially does
>
> git diff-index --check --cached $against --
>
Thank you, I needed something like this, I think I'm going to steal
this to other projects too : )
> where $against is HEAD (or an empty tree object while preparing for
> an initial commit).
>
> > branch_ref = refs_resolve_ref_unsafe(get_main_ref_store(the_repository),
> > "HEAD", 0, NULL, &flags);
> > - if (flags & REF_ISSYMREF)
> > - skip_prefix(branch_ref, "refs/heads/", &branch_name);
> > +
> > + if (flags & REF_ISSYMREF) {
> > + if (skip_prefix(branch_ref, "refs/heads/", &branch_name))
> > + branch_name = branch_name_buf = xstrdup(branch_name);
> > + } else
> > + branch_name = "(no branch)";
>
> Do we need the else clause? The original did not have it and showed
> the "(no branch)" message without an issue, and I do not see anything
> is changed by what happens inside the other side of this if statement.
> Am I missing something?
No, that's just a random idea, I get the point I will remove that in
the new patch
>
> > @@ -1495,6 +1501,7 @@ static int do_create_stash(const struct pathspec *ps, struct strbuf *stash_msg_b
> > strbuf_release(&msg);
> > strbuf_release(&untracked_files);
> > free_commit_list(parents);
> > + free(branch_name_buf);
> > return ret;
> > }
>
> Makes sense.
>
> This is a common pattern we use with a variable whose name contains
> "to_free" (e.g., "branch_name_to_free"), but "branch_name_buf" is
> pleanty readable and easy to understand what is going on.
>
> > +test_expect_success 'stash reflog message uses superproject branch, not submodule branch' '
>
> The title looks a bit on the overly-long side. Would
>
> stash message records the superproject branch
>
> be sufficient? The fact that the stash is implemented as reflog
> is invidible and irrelevant at this level, so "reflog message" is
> wasting bytes without adding any useful information.
>
> What we want to make sure is that the message records the current
> branch name, whether the project has any submodules or not, and from
> that point of view,
>
> stash message records the correct branch name
>
> ought to be good, but not quite, because this test is trying to
> trigger a bug that was present only when there are submodules, so
> not mentioning superproject/submodule at all would not work well.
>
> Would
>
> submodules does not affect the branch recorded in stash message
>
> work? That is the best one I can come up with offhand.
>
Works, I will use this as is, I think this sounds good.
> > + git init sub_project &&
> > + (
> > + cd sub_project &&
> > + echo "Initial content in sub_project" >sub_file.txt &&
> > + git add sub_file.txt &&
> > + git commit -q -m "Initial commit in sub_project"
> > + ) &&
>
> It is easier to debug the test script if you avoid using --quiet too
> much. Regular "sh ./t3903-stash.sh" will squelch these output
> anyway, and they can be seen when the test script is run with "-v".
>
Ok will remove all the -q tests
> > + git init main_project &&
> > + (
> > + cd main_project &&
> > + echo "Initial content in main_project" >main_file.txt &&
> > + git add main_file.txt &&
> > + git commit -q -m "Initial commit in main_project" &&
> > +
> > + git -c protocol.file.allow=always submodule add --quiet ../sub_project sub &&
> > + git commit -q -m "Added submodule sub_project" &&
> > +
> > + git checkout -q -b feature_main &&
>
>
> > + cd sub &&
> > + git checkout -q -b feature_sub &&
> > + cd .. &&
>
> These three lines can be written more compactly as:
>
> git -C sub checkout -b feature_sub &&
>
True.
> > + git checkout -q -b work_branch &&
> > + echo "Important work to be stashed" >work_item.txt &&
> > + git add work_item.txt &&
> > + git stash push -q -m "custom stash for work_branch" &&
> > +
> > + git stash list >../actual_stash_list.txt &&
> > + grep "On work_branch: custom stash for work_branch" ../actual_stash_list.txt
> > + )
> > +'
> > +
> > test_done
>
> Thanks.
Will send a patch soon
Thank you again
- Jayatheerth
