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 V3 1/4] git-mw: Introduction of GitMediawiki.pm
Benoît Person benoit.per...@ensimag.fr writes: 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. Too late ;-). This is a diff/patch thing, and Git is compatible with them. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
Benoît Person benoit.per...@ensimag.fr writes: 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 You can put contrib/mw-to-git/ in your $PATH, and then run git normally. So maybe in the build-perl-script of the toplevel Makefile we could add something copying the script at the toplevel ? I find this a bit dirty, as it polutes the toplevel with untracked files (that are not in the .gitignore file). But that's what the testsuite already does (IIRC, it does a symlink), so I'd say it's OK for now. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
Benoît Person benoit.per...@ensimag.fr writes: 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 :/ . Don't worry, reviews are meant to improve your code (present and future), not to blame you ;-). Just think about what you expect as a user or developer. Would you run git log Makefile or git blame Makefile to know how to use the Makefile? Commit messages are primarily meant for reviewers (here's some code, and here's why it's good and why you should merge it), and can be very useful when bisecting a regression or blaming a source file. Right now, git-remote-mediawiki has only little doc, and the user manual is hosted on a GitHub wiki, not in the source. So there's no ideal place to say how to use the tool as a developer, but a comment in the Makefile should be OK. 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. 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 : 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. I will look into that for the next version ... In any case, using git add -p and if needed its s command avoids introducing unwanted things in the commit. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
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). -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 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). 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. -- Matthieu Moy http://www-verimag.imag.fr/~moy/ -- 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
[PATCH V3 1/4] git-mw: Introduction of GitMediawiki.pm
From: Benoit Person benoit.per...@ensimag.fr
GitMediawiki.pm enable code factoring between several scripts in mw-to-git.
To make it usable in scripts (ie: accessible in @INC) it adds two targets
(copy_pm install_pm) into the Makefile, one for tests and one for
installation.
Signed-off-by: Benoit Person benoit.per...@ensimag.fr
Signed-off-by: Matthieu Moy matthieu@grenoble-inp.fr
---
changes from the V2:
- Add a way to test, without installation, code that uses GitMediawiki.pm.
contrib/mw-to-git/GitMediawiki.pm | 24
contrib/mw-to-git/Makefile| 26 +++---
2 files changed, 47 insertions(+), 3 deletions(-)
create mode 100644 contrib/mw-to-git/GitMediawiki.pm
diff --git a/contrib/mw-to-git/GitMediawiki.pm
b/contrib/mw-to-git/GitMediawiki.pm
new file mode 100644
index 000..8a0ffc7
--- /dev/null
+++ b/contrib/mw-to-git/GitMediawiki.pm
@@ -0,0 +1,24 @@
+package GitMediawiki;
+
+use 5.008;
+use strict;
+use Git;
+
+BEGIN {
+
+our ($VERSION, @ISA, @EXPORT, @EXPORT_OK);
+
+# Totally unstable API.
+$VERSION = '0.01';
+
+require Exporter;
+
+@ISA = qw(Exporter);
+
+@EXPORT = ();
+
+# Methods which can be called as standalone functions as well:
+@EXPORT_OK = ();
+}
+
+1; # Famous last words
\ No newline at end of file
diff --git a/contrib/mw-to-git/Makefile b/contrib/mw-to-git/Makefile
index 1fb2424..b0c7cf2 100644
--- a/contrib/mw-to-git/Makefile
+++ b/contrib/mw-to-git/Makefile
@@ -4,16 +4,36 @@
#
## Build git-remote-mediawiki
+GIT_MEDIAWIKI_PM=GitMediawiki.pm
SCRIPT_PERL=git-remote-mediawiki.perl
GIT_ROOT_DIR=../..
HERE=contrib/mw-to-git/
SCRIPT_PERL_FULL=$(patsubst %,$(HERE)/%,$(SCRIPT_PERL))
+INSTLIBDIR=$(shell $(MAKE) -C $(GIT_ROOT_DIR)/perl \
+-s --no-print-directory instlibdir)
all: build
-build install clean:
+copy_pm:
+ cp $(GIT_MEDIAWIKI_PM) $(GIT_ROOT_DIR)/perl/blib/lib/
+
+install_pm:
+ cp $(GIT_MEDIAWIKI_PM) $(INSTLIBDIR)
+
+build: copy_pm
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+build-perl-script
+
+install: install_pm
$(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
-$@-perl-script
+install-perl-script
+
+clean:
+ $(MAKE) -C $(GIT_ROOT_DIR) SCRIPT_PERL=$(SCRIPT_PERL_FULL) \
+clean-perl-script
+ rm $(INSTLIBDIR)/$(GIT_MEDIAWIKI_PM) \
+ $(GIT_ROOT_DIR)/perl/blib/lib/$(GIT_MEDIAWIKI_PM)
+
perlcritic:
- perlcritic -2 *.perl
+ perlcritic -2 *.perl
\ No newline at end of file
--
1.8.3.GIT
--
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
