Re: [PATCH v4 2/5] unpack-trees: add performance tracing
On Sun, 12 Aug 2018 at 09:19, Nguyễn Thái Ngọc Duy wrote: Hi, > + trace_performance_leave("cache_tree_update"); I would suggest trace_performance_leave() calls use __func__ instead. That way, there's no ambiguity if the function name ever changes. Kindly, Thomas
Re: [PATCH] git-send-email: fix get_maintainer.pl regression
Hi, On Mon, Dec 11, 2017 at 08:46:46PM +0100, Ævar Arnfjörð Bjarmason wrote: > I.e. we'd just ship a copy of Email::Valid and Mail::Address in > perl/Git/FromCPAN/, use a wrapper to load them, and then we wouldn't > need to if/else this at the code level, just always use the module, > and it would work even on core perl. I disagree with the premise of this, Ævar. As soon as you go down this route, it increases maintenance to ensure we keep up to date with what's on CPAN for a tiny edge-case which I don't believe exists. You may as well just use App::FatPacker. We're talking about package maintenance here -- and as I said before, there's plenty of it around. For those distributions which ship Git (and hence also package git-send-email), the dependencies are already there, too. I just cannot see this being a problem in relying on non-core perl modules. Every perl program does this, and they don't go down this route of having copies of various CPAN modules just in case. So why should we? We're not a special snowflake. -- Thomas Adam
Re: [PATCH] git-send-email: fix get_maintainer.pl regression
On Mon, Dec 11, 2017 at 05:13:53PM +, Alex Bennée wrote: > So have we come to a consensus about the best solution here? > > I'm perfectly happy to send a reversion patch because to be honest > hacking on a bunch of perl to handle special mail cases is not my idea > of fun spare time hacking ;-) > > I guess the full solution is to make Mail::Address a hard dependency? This is what I was suggesting, and then as a follow-up, addressing the point that there's a bunch of require() hacks to also get around needing hard-dependencies. -- Thomas Adam
Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input
On Thu, Nov 30, 2017 at 03:12:17PM -0500, Jeff King wrote: > On Wed, Nov 29, 2017 at 06:35:16PM +0000, Thomas Adam wrote: > > > On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schnei...@autodesk.com wrote: > > > + if (print_waiting_for_editor) { > > > + fprintf(stderr, _("hint: Waiting for your editor > > > input...")); > > > fflush(stderr); > > > > Just FYI, stderr is typically unbuffered on most systems I've used, and > > although the call to fflush() is harmless, I suspect it's not having any > > effect. That said, there's plenty of other places in Git which seems to > > think > > fflush()ing stderr actually does something. > > I'd prefer to keep them (including this one), even if they are noops on > most platforms, because: > > 1. They serve as a note for readers of the code that it's important > for the output to have been printed immediately. > > 2. We build on some funny and antique platforms. I wouldn't be > surprised if there's one that line buffers by default. Or even a > modern system with funny settings (e.g., using the GNU stdbuf > tool). > > (I know you said later you don't think this case needs to be removed, > but I want to make it clear I think it's a reasonable project-wide > policy to not assume we we know how stderr is buffered). We're talking past each other, Peff. I'm agreeing with you. I was surprised to see the introduction of fflush(stderr) in the interdiff, when it wasn't present before, was curious to understand why. I've done that, and since stated it's fine to leave it as-is. -- Thomas Adam
Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input
On Thu, Nov 30, 2017 at 02:55:35PM +0100, Lars Schneider wrote: > > > On 29 Nov 2017, at 19:35, Thomas Adam <tho...@xteddy.org> wrote: > > > > On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schnei...@autodesk.com wrote: > >> + if (print_waiting_for_editor) { > >> + fprintf(stderr, _("hint: Waiting for your editor > >> input...")); > >>fflush(stderr); > > > > Just FYI, stderr is typically unbuffered on most systems I've used, and > > although the call to fflush() is harmless, I suspect it's not having any > > effect. That said, there's plenty of other places in Git which seems to > > think > > fflush()ing stderr actually does something. > > I agree with the "unbuffered" statement. I am surprised that you expect > fflush() > to do nothing in that situation... but I am no expert in that area. Can you > point me to some documentation? Because stderr is unbuffered, it will get printed immediately. > In any way, would all this be a problem here? The worst that could happen > would > be that the user would not see the message, right? Correct -- I only bring this up because your interdiff showed you added the fflush() call and I was merely pointing that out. I don't expect you to change it. > Are you aware of stderr usage in Git that could cause more trouble? No. It'll all be fine. -- Thomas Adam
Re: [PATCH v4 0/2] launch_editor(): indicate that Git waits for user input
On Wed, Nov 29, 2017 at 03:37:50PM +0100, lars.schnei...@autodesk.com wrote: > + if (print_waiting_for_editor) { > + fprintf(stderr, _("hint: Waiting for your editor > input...")); > fflush(stderr); Just FYI, stderr is typically unbuffered on most systems I've used, and although the call to fflush() is harmless, I suspect it's not having any effect. That said, there's plenty of other places in Git which seems to think fflush()ing stderr actually does something. -- Thomas Adam
Re: [PATCH] git-send-email: fix get_maintainer.pl regression
On Wed, Nov 22, 2017 at 09:05:41AM +, Alex Bennée wrote: > My hacky guess about GIT's perl use calls is: > >find . -iname "*.perl" -or -iname "*.pm" -or -iname "*.pl" | xargs grep -h > "use .*::" | sort | uniq | wc -l >88 So let us concentrate just on git-send-email.perl for now. In the Module::Extract::Use module (which happens to be what corelist uses), there's an example script called 'extract_modules' which will statically analyse a perl file and tell you the following information: % perl extract_modules ./git-send-email.perl Modules required by ./git-send-email.perl: - Authen::SASL - Cwd (first released with Perl 5) - Email::Valid - Error - File::Spec::Functions (first released with Perl 5.00504) - File::Temp (first released with Perl 5.006001) - Getopt::Long (first released with Perl 5) - Git - Git::I18N - IO::Socket::SSL - MIME::Base64 (first released with Perl 5.007003) - MIME::QuotedPrint (first released with Perl 5.007003) - Net::Domain (first released with Perl 5.007003) - Net::SMTP (first released with Perl 5.007003) - Net::SMTP::SSL - POSIX (first released with Perl 5) - Sys::Hostname (first released with Perl 5) - Term::ANSIColor (first released with Perl 5.006) - Term::ReadLine (first released with Perl 5.002) - Text::ParseWords (first released with Perl 5) - strict (first released with Perl 5) - warnings (first released with Perl 5.006) Therefore, we have the following modules which are not standard: - Email::Valid - Error - Git - Git::I18N - IO::Socket::SSL - NET::SMTP::SSL Looking at the code for git-send-email.perl, it seems most of those are eval()d at the point they're needed, which seems in many cases to be fallback responses to something we've written, or a means of ensuring we don't need to explicitly handle the case of it not being present at run-time. > Should the solution be to just make Mail::Address a hard dependency and > not have the fallback? This seems like a slight on ensuring a running script which may or may not have additional functionality depending on which modules are installed. Given the pretty good state of packaging across those platforms which Git runs on, I would argue we're now in a much better position to explicitly check for non-core modules at BEGIN{} time, and moan loudly if they're not installed. -- Thomas Adam
Re: [PATCH] git-send-email: fix get_maintainer.pl regression
On Tue, Nov 21, 2017 at 08:46:59PM +, Alex Bennée wrote: > > Eric Sunshine <sunsh...@sunshineco.com> writes: > > Aside from those observations, it looks like the tokenizer in this > > function is broken. For any input with the address enclosed in "<" and > > ">", the comment is lost entirely; it doesn't even end up in the > > @tokens array. Since you're already fixing bugs/regressions in this > > code, perhaps that's something you'd like to tackle as well in a > > separate patch? ("No" is an acceptable answer, of course.) > > > >> } elsif ($token eq "<") { > >> push @phrase, (splice @address), (splice @buffer); > >> } elsif ($token eq ">") { > > I can have a go but my perl-fu has weakened somewhat since I stopped > having to maintain perl code for a living. It's almost as though my > brain was glad to dump the knowledge ;-) > > I guess we could maintain a nesting count and a current token type and > use that to more intelligently direct the nested portions to the > appropriate bits. Maybe Matthieu or Remi (CC'ed) might want to chime in > on other options? Trying to come up with a reinvention of regexps for email addresses is asking for trouble, not to mention a crappy rod for your own back. Don't do that. This is why people use Mail::Address. https://metacpan.org/pod/distribution/MailTools/lib/Mail/Address.pod -- Thomas Adam
Re: [PATCH v4 3/7] remote-mediawiki: show known namespace choices on failure
On Mon, Nov 06, 2017 at 04:19:49PM -0500, Antoine Beaupré wrote: > If we fail to find a requested namespace, we should tell the user > which ones we know about, since those were already fetched. This > allows users to fetch all namespaces by specifying a dummy namespace, > failing, then copying the list of namespaces in the config. > > Eventually, we should have a flag that allows fetching all namespaces > automatically. > > Reviewed-by: Antoine Beaupré <anar...@debian.org> > Signed-off-by: Antoine Beaupré <anar...@debian.org> > --- > contrib/mw-to-git/git-remote-mediawiki.perl | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl > b/contrib/mw-to-git/git-remote-mediawiki.perl > index a1d783789..6364d4e91 100755 > --- a/contrib/mw-to-git/git-remote-mediawiki.perl > +++ b/contrib/mw-to-git/git-remote-mediawiki.perl > @@ -1334,7 +1334,8 @@ sub get_mw_namespace_id { > my $id; > > if (!defined $ns) { > - print {*STDERR} "No such namespace ${name} on MediaWiki.\n"; > + my @namespaces = map { s/ /_/g; $_; } sort keys %namespaces_id; Oops. This was my typo from my original suggestion. The hash is '%namespace_id', not '%namespaces_id'. However, how did this slip through testing? I'm assuming you blindly copied this from my example, which although quick to do, is only being caught because of my sharp eyes... -- Thomas Adam
Re: [PATCH v4 2/7] remote-mediawiki: allow fetching namespaces with spaces
On Mon, Nov 06, 2017 at 04:19:48PM -0500, Antoine Beaupré wrote: > From: Ingo Ruhnke <grum...@gmail.com> > > we still want to use spaces as separators in the config, but we should > allow the user to specify namespaces with spaces, so we use underscore > for this. > > Reviewed-by: Antoine Beaupré <anar...@debian.org> > Signed-off-by: Antoine Beaupré <anar...@debian.org> > --- > contrib/mw-to-git/git-remote-mediawiki.perl | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl > b/contrib/mw-to-git/git-remote-mediawiki.perl > index 5ffb57595..a1d783789 100755 > --- a/contrib/mw-to-git/git-remote-mediawiki.perl > +++ b/contrib/mw-to-git/git-remote-mediawiki.perl > @@ -65,6 +65,7 @@ chomp(@tracked_categories); > > # Just like @tracked_categories, but for MediaWiki namespaces. > my @tracked_namespaces = split(/[ \n]/, run_git("config --get-all > remote.${remotename}.namespaces")); > +for (@tracked_namespaces) { s/_/ /g; } > chomp(@tracked_namespaces); Depending on the number if namespaces returned, it might be easier to convert this to the following: my @tracked_namespaces = map { chomp; s/_/ /g; $_; } split(/[ \n]/, run_git("config --get-all remote.${remotename}.namespaces")); This would, once again, avoid creating @tracked_namespaces, and iterating over it. Note that this isn't about trying to 'golf' this; it's a performance consideration. Kindly, Thomas Adam
Re: [PATCH 3/4] remote-mediawiki: show known namespace choices on failure
On Sun, Oct 29, 2017 at 12:08:56PM -0400, Antoine Beaupré wrote: > if we fail to find a requested namespace, we should tell the user > which ones we know about, since we already do. this allows users to > feetch all namespaces by specifying a dummy namespace, failing, then > copying the list of namespaces in the config. > > eventually, we should have a flag that allows fetching all namespaces > automatically. > > Reviewed-by: Antoine Beaupré <anar...@debian.org> > Signed-off-by: Antoine Beaupré <anar...@debian.org> > --- > contrib/mw-to-git/git-remote-mediawiki.perl | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl > b/contrib/mw-to-git/git-remote-mediawiki.perl > index fc48846a1..07cc74bac 100755 > --- a/contrib/mw-to-git/git-remote-mediawiki.perl > +++ b/contrib/mw-to-git/git-remote-mediawiki.perl > @@ -1334,7 +1334,9 @@ sub get_mw_namespace_id { > my $id; > > if (!defined $ns) { > - print {*STDERR} "No such namespace ${name} on MediaWiki.\n"; > + my @namespaces = sort keys %namespace_id; > + for (@namespaces) { s/ /_/g; } I am sure we can improve upon the need to process @namespaces twice: my @namespaces = map { s/ /_/g; $_; } sort keys %namespaces_id; -- Thomas Adam
Re: [PATCH v3 7/7] remote-mediawiki: show progress while fetching namespaces
On Thu, Nov 02, 2017 at 07:10:17PM -0400, Antoine Beaupré wrote: > Actually, is there a standard way to do this in git with Perl > extensions? I know about "option verbosity N" but how should I translate > this into Perl? Carp? Warn? Log::Any? Log4perl? No, not really. From a quick glance at some of the existing perl code in git, a lot of it continues to use "print STDERR" -- but then to be fair, a lot of the perl code also reads like it has been written by C programmers... While there's nothing wrong with using "print STDERR", it's probably wiser to transition this to using Carp in the long run -- it would decrease the round-trip time to debugging should there be a situation where that was needed, and hence I would recommend using "warn" for less-severe errors/debugging. -- Thomas Adam
Re: [PATCH v3 7/7] remote-mediawiki: show progress while fetching namespaces
On Thu, Nov 02, 2017 at 06:26:43PM -0400, Antoine Beaupré wrote: > On 2017-11-02 22:18:07, Thomas Adam wrote: > > Hi, > > > > On Thu, Nov 02, 2017 at 05:25:18PM -0400, Antoine Beaupré wrote: > >> +print {*STDERR} "$#{$mw_pages} found in namespace > >> $local_namespace ($namespace_id)\n"; > > > > How is this any different to using warn()? I appreciate you're using a > > globbed filehandle, but it seems superfluous to me. > > It's what is used everywhere in the module, I'm just tagging along. > > This was discussed before: there's an issue about cleaning up the > messaging in that module, that can be fixed separately. Understood. That should happen sooner rather than later. -- Thomas Adam
Re: [PATCH v3 7/7] remote-mediawiki: show progress while fetching namespaces
Hi, On Thu, Nov 02, 2017 at 05:25:18PM -0400, Antoine Beaupré wrote: > +print {*STDERR} "$#{$mw_pages} found in namespace $local_namespace > ($namespace_id)\n"; How is this any different to using warn()? I appreciate you're using a globbed filehandle, but it seems superfluous to me. Kindly, Thomas
Ascertaining amount of "original" code across files/repo
Hi all, I was recently left with an interesting problem of looking at a heuristic to determine how much original code was left in a repository. Or to put another way, how much the code had changed since. In my case "original code" means "since the initial commit", as this code base had been imported from CVS long ago; and that was the correct starting point. What I came up with was the following heuristic. What I'm curious to know is whether there's an alternative way to look at this and/or if such tooling already exists. What I did was first of all ascertain the number of original lines in each of the files I was interested in: for i in *.[ch] do c="$(git --no-pager blame "$i" | grep -c '^\^')" [ $c -gt 0 ] && echo "$i:$c" done | sort -t':' -k2 -nr Given this, I then did some maths on the total lines from each of those files and to work out a percentage by file, and over all. What I'm curious to know is whether this approach of using "git blame" is a good approach or not. Thanks for your time. -- Thomas Adam
Re: When does git check for branch-X being uptodate with origin/branch-X?
On 21 March 2016 at 21:18, Jeff King <p...@peff.net> wrote: > Again, just my opinion, but that looks awfully clunky. And it doesn't > address the other messages (you might be behind origin/branch-X by N > commits, or ahead by N, but only as of that particular date). Do we want > to annotate every message whose value was computed based on a tracking > branch? Hmm. I would hope not. I'll wait for others to make a call on this, but it's about all I can suggest without significantly bloating the message which isn't desirable either. >> No one's suggesting that this message is removed, I'm not sure where >> you got that from? > > You said earlier: > >> [...]it's more a case of whether even printing that message is useful? > > I didn't know quite what you had in mind, which is why I asked. If we > all agree that removing it is a bad idea, then good, we don't have to > bother discussing that option. :) Ah, oops! I was meaning more, whether to print the message in the case where the branch was uptodate, but now I appreciate it's cached. Apologies for the confusion. -- Thomas Adam -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: When does git check for branch-X being uptodate with origin/branch-X?
On 21 March 2016 at 20:50, Jeff King <p...@peff.net> wrote: > But that's just my opinion. Did you have some specific change you're > interested in? I don't think removing that message is productive; it > _is_ useful information. Perhaps it could be more clear that we are > talking about the tracking branch? I don't have a specific change in mind per-se, rather than to discuss how we might be able to improve the error message, or document somewhere that it's referring to the tracking branch. Maybe that's the point--is it worth mentioning the time/date of when the cache was last updated? That is: "branch-X is uptodate with origin/branch-X (as of DD-MM-YY HH:MM:SS)" No one's suggesting that this message is removed, I'm not sure where you got that from? -- Thomas Adam -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: When does git check for branch-X being uptodate with origin/branch-X?
On 21 March 2016 at 20:28, Jeff King <p...@peff.net> wrote: > We never contact other repositories unless explicitly asked to by > fetch, pull, push, etc. If you want to have the most up-to-date value > without merging, you can just "git fetch" to update the tracking > branches. Thanks. I understand how to use git-fetch, it's more a case of whether even printing that message is useful? I appreciate it can only go on the cached value, but it is still misleading to print that in the case where the cache might not be up to date. Of course, determining that is a different problem. -- Thomas Adam -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
When does git check for branch-X being uptodate with origin/branch-X?
Hi all, Something I've seen a few times of late (although I doubt that's any indication that the code has changed in Git) is the reporting of branch-X being uptodate with origin/branch-X when it isn't. When does git check to see if branch-X has a remote tracking branch and that it has changes on it? Only, the output below is misleading: % git checkout master Switched to branch 'master' Your branch is up-to-date with 'fvwmorg/master'. [fvwm-cvs-to-git/docs]{10345}[0][master] % git pull remote: Counting objects: 26, done. remote: Compressing objects: 100% (12/12), done. remote: Total 26 (delta 18), reused 22 (delta 14), pack-reused 0 Unpacking objects: 100% (26/26), done. >From github.com:fvwmorg/fvwm c029868..36cc898 master -> fvwmorg/master 4f0c7ec..36cc898 ta/git-docs -> fvwmorg/ta/git-docs Updating c029868..36cc898 Fast-forward [...] Clearly, it's obvious that "Your branch is up-to-date with 'fvwmorg/master'." is misleading. Note that in this case, there's no passwords or other hindrances to Git being able to work out that a branch is behind another. Any light that can be shed on this, is much appreciated! Kindly, Thomas Adam -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC] Code reorgnization
On 17 March 2016 at 11:11, Duy Nguyen <pclo...@gmail.com> wrote: > Git's top directory is crowded and I think it's agreed that moving > test-* to t/helper is a good move. I just wanted to check if we could > take this opportunity (after v2.8.0) to move some other files too. I > propose the following new subdirs I wonder whether previous discussions on this still count? See: http://marc.info/?l=git=129650572621523=1 -- Thomas Adam -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Question about rerere
On 6 November 2014 11:30, Tristan Roussel trous...@phare.normalesup.org wrote: Hello, I’ve just learnt about rerere and it’s going to make my life so much easier, I have a question though. I enabled rerere very lately and I wanted to know if there was a way to feed rerere with old merge conflict resolutions (and if not, would it be considered a good feature request)? I’d like to do a rebase of my work because I unintentionally merged a branch I didn’t want and the merge is a bit far in the git history and I don’t want to re-resolve merge conflicts I had after that. Have a look at contrib/rerere-train.sh -- Thomas Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v1 1/2] Remove 'git archimport'
On 9 May 2014 18:52, Felipe Contreras felipe.contre...@gmail.com wrote: Such incredible double standards. I think I speak for everyone when I say: fuck off. I'm sick and tired of seeing you pop up here with polemic and rhetoric, publicly outing those who are making Git great, just because you're not getting your own way. You have a fork of git which you created, go away and use it. -- Thomas Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] contrib/diff-highlight: multibyte characters diff
On 12 February 2014 20:59, Jeff King p...@peff.net wrote: +sub decode { + my $orig = shift; + my $decoded = eval { decode_utf8($orig, Encode::FB_CROAK) }; + return defined $decoded ? I'd still advocate checking $@ here, rather than the defined $decoded check. + ($decoded, sub { encode_utf8(shift) }) : + ($orig, sub { shift }); +} + -- Thomas Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] status: display the SHA1 of the commit being currently processed
Hi, [I rarely do reviews on this list, so feel free to ignore this.] On 17 June 2013 13:10, Mathieu Lienard--Mayor mathieu.lienard--ma...@ensimag.imag.fr wrote: diff --git a/wt-status.c b/wt-status.c index bf84a86..5f5cddf 100644 --- a/wt-status.c +++ b/wt-status.c @@ -885,8 +885,19 @@ static void show_rebase_in_progress(struct wt_status *s, struct wt_status_state *state, const char *color) { + char *stopped_sha = read_line_from_git_path(rebase-merge/stopped-sha); + int must_free_stopped_sha = 1; struct stat st; + /* +* If the file stopped-sha does not exist +* we go back to the old output saying a commit +* instead of providing the commit's SHA1. +*/ + if (!stopped_sha) { + stopped_sha = a commit; + must_free_stopped_sha = 0; + } Rather than having to assign a toggle of deciding when to free stopped_sha, how much overhead would be introduced by just doing: if (!stopped_sha) stopped_sha = strdup(a commit); And then at the end just do: free (stopped_sha); Just a thought. -- Thomas Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Documentation/CommunityGuidelines
Hello, On Mon, Jun 10, 2013 at 06:58:47PM +0530, Ramkumar Ramachandra wrote: I've tried to write down a bare minimum, without restating the obvious. [...] I often come across so-called community guidelines in other projects---some of which adhere to them quite strictly, and others simply document something for the curious. But usually the reason for their existence in the first place are tell-tale signs of trying to fix a problem at the wrong end, and I believe this is what is about to happen if a document such as this ever becomes official. There's no disputing the fact that over the last few weeks, FC's behaviour has been called in to question. He's managed to rub a lot of core people up the wrong way, and in doing so has deliberately side-stepped that problem by doing the one thing which puts anyone trying to raise that point muted; by assuming that he's right. It's a point on which one is never going to win, because no matter what one says, it'll just get twisted round in such a way that one then ends up questioning their own words, and their own conduct, and that's bad, because there never was anything wrong with them to begin with. So when you realise this point, it becomes almost impossible to proceed further with any kind of discussion, because even the technical points of discussion then end up being lost in a tirade of needless side-stepping discussion. It's a bullying tactic on the part of FC which means he'll do any, and everything, to get his own way. So I say to all those seasoned reviewers out there on this list not to put up with it. There comes a point, regardless of how useful someone's contribution may be, that if the barrier to entry is so high that any kind of criticism or comment made against code comes with a massive chance of having to defend yourself against innocence on the part of the reviewer, that those contributions should be shelved. I've seen also another yardstick used to defend FC's behaviour, and that is one of commits within the last three months. That count is completely meaningless, since the review process is always going to be the same. So these guidelines gain the community nothing, and only serve to punish those who are already following them, without them being written down, because the root-cause of the problem is still here, and isn't going to go away, no matter how much referring to these guidelines might help. That is why I think this is the wrong thing to do. -- Thomas Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3] Add new @ shortcut for HEAD
On 1 May 2013 11:12, Eric Sunshine sunsh...@sunshineco.com wrote: On Wed, May 1, 2013 at 5:51 AM, Felipe Contreras felipe.contre...@gmail.com wrote: So HEAD@{0}~0^0 is too much to type, but we can remove '^0', and we can remove '~0', and we can remove 'HEAD', which leaves us with @{0}, but we can't remove '{0}'? This patch allows '@' to be the same as 'HEAD'. So now we can use 'git show @~1', and all that goody goodness. Until now '@' was a valid name, but it conflicts with this idea, so lets s/lets/let's/ (contraction of let us) Ah, the contraction versus the first person singular. In this case where the context is concluding in decision, rather than making a statement (Let's go to the shops, for example) then lets is the correct word to use here. -- Thomas Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 0/1] status: Allow for short-form output by default
Hi, It was asked recently whether git status could output the short-form instead of the long output (via its -sb options). To that end, I've created a rough POC on how this might look. It's deliberately lacking documentation; I was curious to know whether: status.shortwithbranch = true Was going to cut it. Likewise, I was unsure if instead there should be two separate options, one for just short (i.e. '-s') also? What do others think? ISTR reading on this list recently the addition of a --long option to git status? I'm wondering how this would relate to that, if at all? Kindly, Thomas Adam (1): status: Allow for short-form via config option builtin/commit.c | 12 1 file changed, 12 insertions(+) -- 1.7.11.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RFC PATCH 1/1] status: Allow for short-form via config option
It is currently not possible to use the short-form output of git status without declaring an alias to do so. This isn't always desirable therfore, define a git config option which can be set to display the short-form: status.shortwithbranch Signed-off-by: Thomas Adam tho...@xteddy.org --- builtin/commit.c | 12 1 file changed, 12 insertions(+) diff --git a/builtin/commit.c b/builtin/commit.c index a17a5df..552a9f1 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1142,6 +1142,18 @@ static int git_status_config(const char *k, const char *v, void *cb) return error(_(Invalid untracked files mode '%s'), v); return 0; } + + if (!strcmp(k, status.shortwithbranch)) { + if (git_config_bool(k, v)) { + if (!v) + return config_error_nonbool(k); + else if(!strcmp(v, true)) { + status_format = STATUS_FORMAT_SHORT; + s-show_branch = 1; + } + return 0; + } + } return git_diff_ui_config(k, v, NULL); } -- 1.7.11.4 -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4 00/13] New remote-hg helper
On 2 November 2012 18:39, Felipe Contreras felipe.contre...@gmail.com wrote: I disagree. The open source process works not by making favors to each other, but by everyone sharing and improving the code, by *collaborating*. I review your code if you review mine, or if you by me a bear in the next conference is not the spirit of open source, although it might happen often. So shunning any attempt at explanation, and peddling your own thoughts over and over again, irrespective of whether you contribute code or not -- doesn't mean to say you're right, Felipe. And that's the fundamental issue here -- your code speaks for itself, sure, no one denies that, but the code is not even *half* of what makes up the discussion. And so far, the surrounding context and attitude from you doesn't help or enhance the process under which your code is reviewed. And no, you cannot philosophise this, or wriggle out of it through idealism or some other charter or code of conduct -- as reviewers of your code, we have to interact with you to be able to better it. But you seem very reluctant to do that. The fact that we're even having the conversation is evident of that. -- Thomas Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Wrap commit messages on `git commit -m`
Hi, On 1 November 2012 16:07, Ramkumar Ramachandra artag...@gmail.com wrote: Hi, Some of my colleagues are lazy to fire up an editor and write proper commit messages- they often write one-liners using `git commit -m`. However, that line turns out to be longer than 72 characters, and the resulting `git log` output is ugly. So, I was wondering if it would be a good idea to wrap these one-liners to 72 characters automatically. Can't you do this already? From git-log(1): %w([w[,i1[,i2]]]): switch line wrapping, like the -w option of git-shortlog(1). -- Thomas Adam -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html