Re: [PATCH v8 3/7] git-remote-mediawiki: New git bin-wrapper for developement
On 5 July 2013 09:04, Matthieu Moy matthieu@grenoble-inp.fr wrote: benoit.per...@ensimag.fr writes: --- a/contrib/mw-to-git/Makefile +++ b/contrib/mw-to-git/Makefile @@ -2,6 +2,12 @@ # Copyright (C) 2013 # Matthieu Moy matthieu@imag.fr # +# To build and test: +# +# make: +# bin-wrapper/git mw preview Some_page.mw +# bin-wrapper/git clone mediawiki::http://example.com/wiki/ +# The colon after make and the indentation look weird. Shouldn't this be +# To build and test: +# +# make +# bin-wrapper/git mw preview Some_page.mw +# bin-wrapper/git clone mediawiki::http://example.com/wiki/ +# ? oops, yes definitely. -- 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 v7 2/7] wrap-for-bin: Make bin-wrappers chainable
For now, GITPERLLIB is only used in that kind of statements: use lib (split(/:/, $ENV{GITPERLLIB} || ... )); The trailing ':' does not really matter, split will ignore it. That may be true with the current use, but For now leaves funny taste, doesn't it? definitely. For me, the issue was that we were trading that funny taste for less readable code in wrap-for-bin.sh but with that default-substitution construct, it seems worth it. (a huge if-block seemed even funnier in fact :) ) It is not too much trouble to fix by writing GITPERLLIB='@@BUILD_DIR@@/perl/blib/lib'${GITPERLLIB:+:$GITPERLLIB} and we will not have to be worried about future changes breaking today's assumption. That construct is really nice :) did not know there was such a thing in bash, thanks. -- 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 v7 2/7] wrap-for-bin: Make bin-wrappers chainable
Do we want to add that ':' unconditionally? Could GITPERLLIB be empty? For now, GITPERLLIB is only used in that kind of statements: use lib (split(/:/, $ENV{GITPERLLIB} || ... )); The trailing ':' does not really matter, split will ignore it. With the current codebase, I think it's nicer to do it that way (makes wrap-for-bin.sh more readable). -- 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 v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement
GIT_ROOT_DIR=../../.. GIT_EXEC_PATH=$(cd $(dirname $0) cd ${GIT_ROOT_DIR} pwd) GIT_MEDIAWIKI=$GIT_EXEC_PATH/contrib/mw-to-git PATH=$GIT_MEDIAWIKI/contrib/mw-to-git:$PATH GPLEXTRA=$GIT_MEDIAWIKI/contrib/mw-to-git exec ${GIT_EXEC_PATH}/bin-wrappers/git $@ Should I do that in a reroll ? Two possible alternatives: - Is there a reason you would not want to install whatever Perl modules you want to use via GITPERLLIB mechanism to ../../perl/blib/lib? If we are making modifications to Git/Mediawiki.pm or even git-mw.perl / git-remote-mediawiki.perl, installing them without proper testing beforehand seems wrong. That's not installing (i.e. not make install), just a copy within the source tree. Same as what's done in Git's toplevel. Oh right, sorry, read it too quickly :/ Well, I still find it weird to copy untested files out of contrib/mw-to-git/ though. Another idea crossed my mind: for now the test suite creates a symlink of git-remote-mediawiki in the toplevel if it's not installed. It would be better to use the bin-wrapper in the testsuite I think ? (plus, with the current solution, it may have problems finding the latest Git::Mediawiki.pm) -- 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: contrib/mw-to-git/Git/Mediawiki.pm
Oops, so sorry :/ It's defintely doable since the lowercase 'git' is only a bin-wrapper for git to ease development in contrib/mw-to-git/ . Junio, Matthieu : should I resend a new version of my serie which renames the 'git' (lowercase) file into something like 'git-dev' ? (some comments directly mentionning the 'git' (lowercase) file needs to be updated as well in the Makefile) Benoit -- 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: contrib/mw-to-git/Git/Mediawiki.pm
Hmph. Does it even need to be in-tree then? Is it insufficient to run ../../git from that directory instead? Well, the fact is we use Perl packages now (Git.pm and Git::Mediawiki.pm in contrib/mw-to-git/Git/). The way we build perl scripts in the toplevel's Makefile makes those packages accessible only in $GITPERLLIB if it's defined and defaults to $INSTLIBDIR to seek for installed version of those packages. We use a bin-wrapper to define that $GITPERLLIB variable so that the installed version gets bypassed by the one presents in the directory defined in $GITPERLLIB. I just noticed that the script is not strictly a text file, ending with an incomplete line, by the way. an incomplete line ? -- 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 v6 2/5] git-remote-mediawiki: new git bin-wrapper for developement
As far as I can tell, the only real reason why you need this and cannot use ../../bin-wrappers/git directly is because the GITPERLLIB it gives you only points at ../../perl/blib/lib and not this directory. Plus (forgot to mention it in the other mail :/ ) it enables us to not copy git-mw and git-remote-mediawiki at each make and stop us from polluting the toplevel directory with those two scripts during development. Two possible alternatives: - Is there a reason you would not want to install whatever Perl modules you want to use via GITPERLLIB mechanism to ../../perl/blib/lib? If we are making modifications to Git/Mediawiki.pm or even git-mw.perl / git-remote-mediawiki.perl, installing them without proper testing beforehand seems wrong. Perhaps it will interfere with the real installation step in ../../perl/Makefile? I don't think so, but I am not an expert on what's going on in ../../perl/Makefile. If that is the case, then it is not a good idea, but otherwise, that would let you use ../../bin-wrappers/git as-is. - Perhaps we could do: GITPERLLIB=${GPLEXTRA+$GPLEXTRA:}@@BUILD_DIR@@/perl/blib/lib in wrap-for-bin.sh, so that your instruction can become GPLEXTRA=$(pwd) ../../bin-wrappers/git whatever-mw-thing and you do not have to have this file? We would also need to unset GPLEXTRA at the beginning of test-lib.sh if we were to do this. How does a developer (or user) use this script? From your Makefile (e.g. make test)? Manually following some written instruction? In either case, the latter option feels a lot more sensible alternative without having to maintain this extra copy of wrap-for-bin that can easily go out of sync. A developer uses it like any bin-wrapper : bin-wrapper/git mw preview for instance. He can add it to its path while working on the scripts etc ... The best way to do it would be to factorise those new use cases (adding new elements in $GITPERLLIB and $PATH) in the code that generate bin-wrappers from wrap-for-bin.sh I think. But it was weird to work on that in this serie which initial goal was to add a preview tool for git-remote-mediawiki and ended up adding a new perl package, a new 'git mw' command which handles subcommands, ... In the end, this patch could be removed from the serie since it is self-contained : it is not used by 3/5, 4/5, and 5/5. Benoit -- 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 0/5] git-remote-mediawiki: new tool to preview local changes without pushing
Ok, thanks for all the reviews, Final version (I hope :) ) will come when all the mediawiki patches will have graduated to 'master' then. Benoit Person -- 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 1/4] git-mw: Introduction of GitMediawiki.pm
On 17 June 2013 09:12, Matthieu Moy matthieu@grenoble-inp.fr wrote: Also, it seems to be only part of the solution. With your change, from contrib/mw-to-git/ and after running only make, ./git-mw takes the installed version of GitMediawiki.pm in priority ../../bin-wrappers/git takes the installed version of git-mw only (i.e. does not know git mw if make install hasn't been ran). Same thing as the documentation point, I think I am a bit lost in that whole thing. I will re-look into it for the next version :/ . In short, the include path should contain both the *.pm file and the git-foo ones. The fact is, for now, is there a way to test changes in git-remote-mediawiki.perl without 'make install'-ing it ? I could not find one So maybe in the build-perl-script of the toplevel Makefile we could add something copying the script at the toplevel ? And in GitMediawiki's Makefile, we let everything stay as is : copying *.pm into /perl/blib/lib when building and copying it in installdir when installing ? I think you removed a newline from the end of the file. It's usually considered good practice to have this trailing newline (e.g. so that cat file in a terminal doesn't put your prompt after the last line). IIRC, it's actually required to call the file a text file according to POSIX. That catch oO, thanks for the explanations. From my point of view, this could definitely be improved from: - perlcritic -2 *.perl + perlcritic -2 *.perl \ No newline at end of file to something like that: - perlcritic -2 *.perl + perlcritic -2 *.perl \ removed newline at end of file which gives more insights into why the line is considered edited. Benoit Person -- 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] send-email: allow use of basic email list in --cc --to and --bcc
+sub split_email_list { +my(@list) = @_; +my @tmp; +my @emails; + for (my $i = 0; $i = $#list; $i++) { + if ($list[$i] =~ /,/) { + @emails = split(/,/, $list[$i]); + } else { + @emails = $list[$i]; + } + # Removal of unwanted spaces + for (my $j = 0; $j = $#emails; $j++) { + $emails[$j] =~ s/^\s+//; + $emails[$j] =~ s/\s+$//; + } + @tmp = (@tmp, @emails); + } +return(@tmp); +} Why two regex ? You could do something like : $emails[$j] =~ s/^\s+|\s+$//g; to remove leading and trailing whitespaces at the same time. I think it's better to use the builin 'push' function to concatenate your two arrays: push(@tmp, @emails); Benoit Person -- 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 1/4] git-mw: Introduction of GitMediawiki.pm
On 16 June 2013 22:18, Matthieu Moy matthieu@grenoble-inp.fr wrote: benoit.per...@ensimag.fr writes: changes from the V2: - Add a way to test, without installation, code that uses GitMediawiki.pm. This still needs to be documented, even very quickly, somewhere in the code (e.g a comment in the Makefile). Well, I think I will have to re-read some docs (and some earlier reviews) about what to write in commit messages, in the emails, in the code as comments and in the documentation ... I am just totally lost right now :/ . -build install clean: +copy_pm: + cp $(GIT_MEDIAWIKI_PM) $(GIT_ROOT_DIR)/perl/blib/lib/ I already commented on this: http://thread.gmane.org/gmane.comp.version-control.git/227711/focus=227721 Oops, I forgot that one, so sorry :/ . Also, it seems to be only part of the solution. With your change, from contrib/mw-to-git/ and after running only make, ./git-mw takes the installed version of GitMediawiki.pm in priority ../../bin-wrappers/git takes the installed version of git-mw only (i.e. does not know git mw if make install hasn't been ran). Same thing as the documentation point, I think I am a bit lost in that whole thing. I will re-look into it for the next version :/ . perlcritic: - perlcritic -2 *.perl + perlcritic -2 *.perl \ No newline at end of file Please, avoid these whitespace-only changes. They create noise during review, and more potential conflicts. For that one, I don't know why git assumes there is a change in it : from what I see there is absolutely no whitespaces changes but maybe my editor added some weird character somewhere ? I will look into that for the next version ... Thank you, Benoit Person -- 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/RFC 3/4] git-mw: Adding git-mw.perl script
The V3 is ready, but I am still not sure about what is the best way to do it for this issue though. On 13 June 2013 15:01, Matthieu Moy matthieu@grenoble-inp.fr wrote: benoit.per...@ensimag.fr writes: How does the make Vs make install work? How does a developer run the tool without installing? Well it does not work without installing but I think you know that now :) I first tried: $ ../../bin-wrappers/git mw git: 'mw' is not a git command. See 'git --help'. Then, this first seem OK: $ ./git-mw usage: git mw command args git mw commands are: HelpDisplay help information about git mw Preview Parse and render local file into HTML BUT, this will take the installed GitMediawiki.pm if it is available, and we don't want this (if one hacks GitMediawiki.pm locally, one wants the new hacked to be taken into account without make installing it). To understand better how it works, try adding this in git-mw.perl: print $_\n for @INC; I get this: /home/moy/local/usr-squeeze/share/perl/5.14.2 /home/moy/local/usr-squeeze/src/MediaWiki-API-0.39/blib/lib /etc/perl /usr/local/lib/perl/5.14.2 /usr/local/share/perl/5.14.2 /usr/lib/perl5 /usr/share/perl5 /usr/lib/perl/5.14 /usr/share/perl/5.14 /usr/local/lib/site_perl . The '.' is there, but it comes after the hardcoded /home/moy/local/usr-squeeze/share/perl/5.14.2 (which has to comes first, to let the install version be robust to whatever comes after). Thanks for the explanations I think you need an equivalent of Git's toplevel bin-wrappers/git, or perhaps use the same bin-wrapper/git but let make install in contrib/mw-to-git/ install GitMediawiki.pm in perl/blib/lib Typo s/make install/make/ ? For now, I have implemented that one : each time you do `make`, if there is changes in GitMediawiki.pm, it gets copied to $GITPERLLIB (perl/blib/lib). But I am not sure it's the best approach here. If we want something entirely self-contained for GitMediawiki, creating a new git wrapper seems like the best way. But then, we could say that since GitMediawiki Makefile uses Git toplevel Makefile, it's not entirely self-contained :/ maybe it's the copying file that makes it weird ? BTW, I just noticed we had a Git::SVN, so perhaps GitMediawiki should be Git::MediaWiki. For that one, I am not really sure Git::Mediawiki makes more sense than GitMediawiki. The point of the GitMediawiki.pm package is to contain all the stuff for the bidirectionnal-thingy. So they are not really Git-related, nor Mediawiki-related. Making it part of a Git directory / namespace does not really feels right, even if it's how it's done for SVN :/ . Thank you for the all the reviews, (it works for Ensiwiki now \o/) Benoit Person -- 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/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
The V2 is on the launchpad but I am still struggling with the code factoring between git-mw.perl and git-remote-mediawiki.perl :/ . On 9 June 2013 08:08, Jeff King p...@peff.net wrote: You could make a Git::MediaWiki.pm module, but installing that would significantly complicate the build procedure, and potentially be annoying for users. One trick I have done in the past is to concatenate bits of perl script together in the Makefile, like this: foo: common.pl foo.pl { \ echo '$(PERL_PATH_SQ)' \ for i in $^; do \ echo #line 1 $src \ cat $src \ done \ } $@+ mv $@+ $@ That would conflict a bit with the way we chain to git's Makefile, though. I suspect you could do something complicated like build foo.pl from common.pl and foo-main.pl, then chain to git's Makefile to build foo from foo.pl. I've implemented this one for now but after a real-life meeting with Matthieu Moy we discussed the possibility to build a GitMediawiki.pm module. It seems more clean than the concatenation of perl scripts. Plus, it would force people to limit side effects inside the functions used in this package/utils file (I have in mind the mw_connect_maybe function here and a couple of others which directly *hope* for global vars to be set to a nice value before being called). What I find bad in the concatenating-thingy is the mandatory rename of git-mw.perl into something like git-mw.unmerged.perl and git-remote-mediawiki.perl into git-remote-mediawiki.unmerged.perl. Otherwise, like you said, it would be hard to chain to git's Makefile after the merge. For now, I have really no idea which one is the best. If I may ask, what did you have in mind while saying: You could make a Git::MediaWiki.pm module, but installing that would significantly complicate the build procedure, and potentially be annoying for users. ? Since my previous commit (ea07ec1 in next - use Git.pm functions for credentials), git-remote-mediawiki.perl already depends on the proper installation of the Git.pm package. In what ways the need for the installation of yet another package (GitMediawiki.pm) would annoy a user ? Thank you for your time, Benoit -- 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 22/28] git-remote-mediawiki: Modify strings for a better coding-style
Well, I think next step would be to replace all those calls with Git.pm `command`, `command_oneline` and `config``which take an array of arguments after the command. In the preview tool we use those but I don't know if we will find the time to clean that up too in git-remote-mediawiki :) . Don't know though if it's better to mix that with this serie which is mainly based on what perlcritic returns. On 10 June 2013 18:41, Junio C Hamano gits...@pobox.com wrote: Matthieu Moy matthieu@grenoble-inp.fr writes: Célestin Matte celestin.ma...@ensimag.fr writes: @@ -1285,8 +1285,7 @@ sub get_mw_namespace_id { # Look at configuration file, if the record for that namespace is # already cached. Namespaces are stored in form: # Name_of_namespace:Id_namespace, ex.: File:6. -my @temp = split(/\n/, run_git(config --get-all remote. -. $remotename ..namespaceCache)); +my @temp = split(/\n/, run_git(config --get-all remote.${remotename}.namespaceCache)); I tend to prefer the former, as it avoids long lines ( 80 columns) But you split the name of a single variable across lines by doing so. You could split lines to make it a bit more readable. my @temp = split(/\n/, run_git(config --get-all remote.${remotename}.namespaceCache)); It still overflows the 80-column limit, but the namespaceCache is the only thing that overflows; not much harm is done. This is totally outside of the topic of coding-style series, but I would be more concerned about the use of ${remotename}, though. Has it (and in general the parameters to all calls to run_git()) been sanitized for shell metacharacters? If we had a variant of run_git that took an array of command line arguments given to git, you could do this my @temp = split(/\n/, run_git([qw(config --get-all), remote.${remotename}.namespaceCache)]); which would be safer and easier to read. @@ -1339,8 +1338,7 @@ sub get_mw_namespace_id { # Store explicitely requested namespaces on disk if (!exists $cached_mw_namespace_id{$name}) { -run_git(config --add remote.. $remotename -..namespaceCache \. $name .:. $store_id .\); +run_git(qq(config --add remote.${remotename}.namespaceCache ${name}:${store_id})); Same. -- 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 -- 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/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
Matthieu Moy matthieu@grenoble-inp.fr writes: Same question here. I'd expect git mw preview in a mediawiki workflow to do what pdflatex foo evince foo.pdf do in a latex workflow: see in rendered form what I've been doing. In a latex flow, if I want to see how my local changes merge with the remote ones, I do git merge pdflatex, and I'd do the same with git mw. In fact, I should not have used merge to describe how the two contents (page template + new parsed content) are combined together. For now, the code simply replaces the template page's text content (the one retrieved from the remote) with the new one. It does not really care if the remote has changes or not. (And, to be honest, I did not thought about that issue ;) ). But, like both of you said : in a typical workflow, the merging would be left to the user so the current behavior is fine I think ? I also wonder if it would be useful to be able to specify not only files in the filesystem, but also arbitrary blobs. So in 4b above, you could git mw preview origin:page.mw to see the rendered version of what upstream has done. Next step could even be git mw diff $from $to, using the wiki to render the diff. Not a priority, but could be funny. I searched in the Mediawiki API if there was a way to diff from a stored revision and raw text content but I've found nothing :/ . We could make a little hack to do that by saving as a new revision the local content, and use the DeleteRevision-thingy from Mediawiki [1] to hide this useless revision but it would floods the remote DB and usually users to not have the permission to use that tool. So, for now I would say it's a no-go :/ . [1] http://www.mediawiki.org/wiki/Manual:RevisionDelete Benoit Person -- 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 -- 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/RFC] git-remote-mediawiki: new tool to preview local changes without pushing
On 9 June 2013 08:08, Jeff King p...@peff.net wrote: I also wonder if it would be useful to be able to specify not only files in the filesystem, but also arbitrary blobs. So in 4b above, you could git mw preview origin:page.mw to see the rendered version of what upstream has done. Hum, so `git mw preview origin:page.mw` would just do the get request to the remote mediawiki, save it locally and - maybe - load it in the browser ? Is it really better than just opening the browser and typing the right URL ? Currently, this URL is one click away when you have preview file loaded in a web browser. It works but a couple of points trouble me: 1- I had to copy two functions from `git-remote-mediawiki.perl`, I don't really know how we could factorize those things ? I don't think it makes much sense to create a package just for them ? You could make a Git::MediaWiki.pm module, but installing that would significantly complicate the build procedure, and potentially be annoying for users. One trick I have done in the past is to concatenate bits of perl script together in the Makefile, like this: foo: common.pl foo.pl { \ echo '$(PERL_PATH_SQ)' \ for i in $^; do \ echo #line 1 $src \ cat $src \ done \ } $@+ mv $@+ $@ That would conflict a bit with the way we chain to git's Makefile, though. I suspect you could do something complicated like build foo.pl from common.pl and foo-main.pl, then chain to git's Makefile to build foo from foo.pl. ok, thanks a lot. 2- The current behavior is to crash if the current branch do not have an upstream branch on a valid mediawiki remote. To find that specific remote, it runs `git rev-parse --symbolic-full-name @{upstream}` which will return something like `/refs/remotes/$remote_name/master`. 2a- maybe there is a better way to find that remote name ? If you just care about the remote name and not the name of the local branch, you can just ask for my $curr_branch = `git symbolic-ref HEAD`; my $remote = `git config branch.$curr_branch.remote`; my $url = `git config remote.$remote.url`; Of course you would want some error checks and probably some chomp()s in there, too. The fact is, `git symbolic-ref HEAD` result would have to be parsed in order to extract the current branch name like I currently extract the remote name. So, is it really better than `git rev-parse --symbolic-full-name @{upstream}` ? 2b- would it be useful to add a fallback if that search fails ? searching for a valid mediawiki remote url in all the remotes returned by `git remote` for instance ? That is probably OK as long as there is only one such remote, and it would help the case where you have branched off of a local branch (so your upstream remote is .). If there are two mediawiki remotes, though, it would make sense to simply fail, as you don't know which to use. But I'd expect the common case by far to be that you simply have one such remote. Well, I thought that `git mw preview` could provide an interactive mode where, if the first search fails, it would find all the mediawiki remotes, and offers to the user a way to choose the remote ? Benoit Person -- 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 -- 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 v2 02/22] git-remote-mediawiki: Use the Readonly module instead of the constant pragma
The major drawback of using perl `constant` is the fact that they do not interpolate like variables in most of the contexts (those who automatically quotes barewords). Like said on Perl Monks here [1], you have to do some tricks depending on the context in which you're using the constant and that's not really clean. But maybe trading clean for another package is not worth it. I just searched for alternatives way of doing it and none seems to be in the core packages so maybe we should just drop this. [1] http://www.perlmonks.org/?node_id=11 On 8 June 2013 05:23, Jeff King p...@peff.net wrote: On Fri, Jun 07, 2013 at 11:42:03PM +0200, Célestin Matte wrote: diff --git a/contrib/mw-to-git/git-remote-mediawiki.perl b/contrib/mw-to-git/git-remote-mediawiki.perl index 4893442..e60793a 100755 --- a/contrib/mw-to-git/git-remote-mediawiki.perl +++ b/contrib/mw-to-git/git-remote-mediawiki.perl @@ -26,21 +26,21 @@ use IPC::Open2; use Readonly; What does this series apply on top of? The existing version in master does not have use Readonly in it at all. The first version of your series introduced that line, but here it is shown as an existing line. Did you miss a commit when putting your patches together? # Mediawiki filenames can contain forward slashes. This variable decides by which pattern they should be replaced -use constant SLASH_REPLACEMENT = %2F; +Readonly my $SLASH_REPLACEMENT = %2F; What advantage does this have over use constant? I do not mind following guidelines from perlcritic if they are a matter of style, but in this case there is a cost: we now depend on the Readonly module, which is not part of the standard distribution. I.e., users now have to deal with installing an extra dependency. Is it worth it? -Peff -- 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 -- 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
contributing to git-mediawiki
Hello, We are another team from Ensimag (Célestin MATTE Benoit PERSON) who will contribute to Git and more specifically to Git-Mediawiki for our one-month school project - and possibly more. We already have a couple of basic patches in local and will submit them in the upcoming days. After that, we will start working on more useful features. As a first start, we thought about these two ideas posted in the Git-Mediawiki’s issue tracker: - A mechanism to preview local modifications in the browser (with the wiki style) -- https://github.com/moy/Git-Mediawiki/issues/7 - Support for “push-by-rev” to increase push performance on some wikis -- https://github.com/moy/Git-Mediawiki/issues/6 If you have any suggestions, ideas, guidances, feel free to share them with us. Best regards, Célestin MATTE Benoit PERSON -- 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