Hi Davo, checklist fork ready #3

jfruitet · 2012-04-11T16:07:12Z

I did all the modifications you suggested, except lang files sorting, but sure it will be done when that fork will be stabilized.
For Mahara stuff I'll set a new branch...
I'll be in vacations next week.
Jean.

Signed-off-by: Jean FRUITET <[email protected]>

davosmith · 2012-04-11T16:08:59Z

Thanks - I'll review and get back to you

upload CheckList as HTML / LEAP2 to Mahara downlod View from mahara as prove of practice Nouvelle branche : déposer une CheckList vers Mahara Télécharger une vue mahara comme preuve de pratique

… item (outcome). table checklist_comment, scrip cron_outcomes.php

…list item are not yet locked. Signed-off-by: Jean FRUITET <[email protected]>

Signed-off-by: Jean FRUITET <[email protected]>

davosmith · 2012-07-07T17:28:21Z

I am really sorry that it has taken me such a long time to get around to reviewing the changes you have made to the checklist module.

I've started looking at the changes this afternoon, but I'm afraid that I'm struggling to find my way through the changes you have made. For me to have any chance of reviewing and integrating this, I'm going to have to ask you to create a cleaned-up version of this, with 1 commit for each new feature, along with a clear commit message explaining exactly what feature you are adding.

Going through the commits, the first commit 'MODIF JF tags removed' adjusts the backup to include data tables that have not been added to the checklist module by that point. It also includes multiple restore_decode_rule statements, all for decoding the same string (CHECKLISTEDITBYCHECKLIST), but with different expected results. This is a clear bug, which you may have fixed in later commits, but it is difficult for me to locate where such a fix is, if it exists.

The next two commits, both called 'Initial commit for JF fork' appear to be the same - are there some subtle differences I should be aware of? If not it would be helpful to include only one of these commits there. These also, unless I have misunderstood them, introduce more than one new feature / bug fix, so it would be much easier to review if they were multiple commits. You have partially documented your changes, but there are changes in files not mentioned in your README file. I notice one of the files has a large amount of commented-out SQL - is this needed?

I'm not clear what the 'Correction bug course id' commit is doing.

The commit 'New branch :' has a non-descriptive commit message. It appears to be related to exporting to Mahara, but I would prefer to have a clear description of what it does and how it does it. It also includes at least one reference to 'mod-referentiel', a number of language strings which are not in alphabetical order (which makes ongoing maintenance much more fiddly) and a number of variables / comments / function names in French (I have nothing against the French language, but as I will need to maintain any integrated code in the future, it is much more helpful if I understand clearly what it all means).

"follow_link" icon added as link ... - this looks fairly straight-forward, but I would prefer redundant code to be removed, rather than commented out (that is why we have version control software like git).

Few bugs corrections about Mahara stuff ... - again, fairly straightforward. Maybe should be integrated back into your original Mahara commit, but not essential, as I can see what is going on there.

Don't lock mahara stuff - this commit only seems to add a comment, that doesn't really explain what is going on.

Signed-off-by: Jean FRUITET ... - looks like an obvious bug fix, but it would be helpful to explain exactly what is being fixed.

I know that what I am asking here is probably a large amount of work. Unfortunately, from my perspective, reviewing the code in the way the commits are currently organised is also a large amount of work. Unless you are able to make my job a lot easier, I am afraid I will not be able to accept these changes. I am sorry if I am being unreasonable in anything I have written and I hope you understand why I am saying this.

jfruitet · 2012-07-09T16:07:40Z

Hi Davo,

I had never used GIT.
Surely the way I did that fork is not the righ one.

I shall delete all that fork and redo a brand new one.
I'll try to do a commit per new feature (but surely not one for each
line of code !;>))

New features type 1:
1.1 - outcomes as list of items (affect import / export)
1.2 - outcomes validation made in any Moodle activity pushed to
Checklist+ (affect cron job)

New features type 2:
2.1 - prove of practice attached to each item (affect table structure)
2.2 - Mahara connection (selected links / export data) (uses portfolio)

Please feel free to give me your keen advice, for wich I'll thank you much.

Regards.
Jean.

P.S.

I have presented the CheckList+ to French MoodleMoot 2012.
http://moodlemoot2012.unimes.fr/
http://moodlemoot2012.unimes.fr/course/view.php?id=50

A lot of people interested by that evolution...

JF.

Le 07/07/2012 19:28, Davo Smith a écrit :

I am really sorry that it has taken me such a long time to get around to reviewing the changes you have made to the checklist module.

I've started looking at the changes this afternoon, but I'm afraid that I'm struggling to find my way through the changes you have made. For me to have any chance of reviewing and integrating this, I'm going to have to ask you to create a cleaned-up version of this, with 1 commit for each new feature, along with a clear commit message explaining exactly what feature you are adding.
OK.
I'll try that.

Going through the commits, the first commit 'MODIF JF tags removed' adjusts the backup to include data tables that have not been added to the checklist module by that point. It also includes multiple restore_decode_rule statements, all for decoding the same string (CHECKLISTEDITBYCHECKLIST), but with different expected results. This is a clear bug, which you may have fixed in later commits, but it is difficult for me to locate where such a fix is, if it exists.

The next two commits, both called 'Initial commit for JF fork' appear to be the same - are there some subtle differences I should be aware of? If not it would be helpful to include only one of these commits there. These also, unless I have misunderstood them, introduce more than one new feature / bug fix, so it would be much easier to review if they were multiple commits. You have partially documented your changes, but there are changes in files not mentioned in your README file. I notice one of the files has a large amount of commented-out SQL - is this needed?

I'm not clear what the 'Correction bug course id' commit is doing.

The commit 'New branch :' has a non-descriptive commit message. It appears to be related to exporting to Mahara, but I would prefer to have a clear description of what it does and how it does it. It also includes at least one reference to 'mod-referentiel', a number of language strings which are not in alphabetical order (which makes ongoing maintenance much more fiddly) and a number of variables / comments / function names in French (I have nothing against the French language, but as I will need to maintain any integrated code in the future, it is much more helpful if I understand clearly what it all means).

"follow_link" icon added as link ... - this looks fairly straight-forward, but I would prefer redundant code to be removed, rather than commented out (that is why we have version control software like git).

Few bugs corrections about Mahara stuff ... - again, fairly straightforward. Maybe should be integrated back into your original Mahara commit, but not essential, as I can see what is going on there.

Don't lock mahara stuff - this commit only seems to add a comment, that doesn't really explain what is going on.

Signed-off-by: Jean FRUITET ... - looks like an obvious bug fix, but it would be helpful to explain exactly what is being fixed.

I know that what I am asking here is probably a large amount of work. Unfortunately, from my perspective, reviewing the code in the way the commits are currently organised is also a large amount of work. Unless you are able to make my job a lot easier, I am afraid I will not be able to accept these changes. I am sorry if I am being unreasonable in anything I have written and I hope you understand why I am saying this.
OK.
That's correct.

Reply to this email directly or view it on GitHub:
#3 (comment)

Aucun virus trouvé dans ce message.
Analyse effectuée par AVG - www.avg.fr
Version: 2012.0.2195 / Base de données virale: 2437/5118 - Date: 08/07/2012

Jean FRUITET - St Sébastien / Loire

davosmith · 2012-07-09T18:12:46Z

Yes - one commit per new feature / bug fix would be great - one commit per
line would be terrible!

I use the command line version of git and this is how I would proceed.

( git config --global color.ui auto // makes the output a lot prettier )

git add remote davosmith
git://github.com/davosmith/moodle-checklist.git // This adds my
official repo as a source for your repo
git fetch davosmith // Get all the latest code from my repo
git checkout davosmith/master // Check out my master branch
git checkout -b mynewbranch // Create a new branch, off my master
branch, to put your code in (give it a better name than that!)
Copy & paste in the code for the first feature / fix (try to avoid
adding trailing whitespace or adjusting the layout of any of the existing
code)
git diff // to check exactly what you have changed - I can also
recommend gitk (if you have it installed) to check
git add FILENAME // for each file you have changed
(optional) use gitk again, to double-check all the files / changes are
queued up correctly; you can also use 'git status' to check the queued
files as well
git commit -m "Brief description of the feature that is being added" //
If it needs more explanation, leave out the -m "Brief description ...." and
type into the text editor that appears a brief description, a blank line,
then a more detailed explanation.
Go back to step 5 until all changes checked in
git push origin mynewbranch:mynewbranch // This pushes 'mynewbranch'
to github and creates a new branch there called 'mynewbranch' (substitute
the branch name you used in step 4)
Go onto github and put in a pull request from 'mynewbranch'

There are some alternatives to 'git add FILENAME' - you can also use 'git
add *.php' (to add all PHP files in the current folder) or 'git add db' (to
add all files in the db folder + sub folders) or 'git add .' (to add
everything in the current folder and subfolders). You can also, skip the
'git add' altogether and call 'git commit -am "brief description"' if you
know exactly what you have changed, as this adds and commits all the
previously known files (but does not add any new files). Use this last
option with great care (I have often committed changes that I didn't want,
or missed new files with this version).

One thing you might want to do BEFORE step 11, is to do another 'git fetch
davosmith' to see if I have changed anything (unlikely) and then a 'git
rebase davosmith/master' to pull in my latest changes and then re-run your
changes over the top. Do not do this after you have pushed to github, as it
won't work (without a --force parameter).

I hope that helps you out a bit with Git - feel free to ask a question if
you get stuck ( [email protected] )

Davo

On Mon, Jul 9, 2012 at 5:07 PM, Jean FRUITET <
[email protected]

wrote:

Hi Davo,

I had never used GIT.
Surely the way I did that fork is not the righ one.

I shall delete all that fork and redo a brand new one.
I'll try to do a commit per new feature (but surely not one for each
line of code !;>))

New features type 1:
1.1 - outcomes as list of items (affect import / export)
1.2 - outcomes validation made in any Moodle activity pushed to
Checklist+ (affect cron job)

New features type 2:
2.1 - prove of practice attached to each item (affect table structure)
2.2 - Mahara connection (selected links / export data) (uses portfolio)

Please feel free to give me your keen advice, for wich I'll thank you much.

Regards.
Jean.

P.S.

I have presented the CheckList+ to French MoodleMoot 2012.
http://moodlemoot2012.unimes.fr/
http://moodlemoot2012.unimes.fr/course/view.php?id=50

A lot of people interested by that evolution...

JF.

Le 07/07/2012 19:28, Davo Smith a écrit :

I am really sorry that it has taken me such a long time to get around to
reviewing the changes you have made to the checklist module.

I've started looking at the changes this afternoon, but I'm afraid that
I'm struggling to find my way through the changes you have made. For me to
have any chance of reviewing and integrating this, I'm going to have to ask
you to create a cleaned-up version of this, with 1 commit for each new
feature, along with a clear commit message explaining exactly what feature
you are adding.
OK.
I'll try that.

Going through the commits, the first commit 'MODIF JF tags removed'
adjusts the backup to include data tables that have not been added to the
checklist module by that point. It also includes multiple
restore_decode_rule statements, all for decoding the same string
(CHECKLISTEDITBYCHECKLIST), but with different expected results. This is a
clear bug, which you may have fixed in later commits, but it is difficult
for me to locate where such a fix is, if it exists.

The next two commits, both called 'Initial commit for JF fork' appear to
be the same - are there some subtle differences I should be aware of? If
not it would be helpful to include only one of these commits there. These
also, unless I have misunderstood them, introduce more than one new feature
/ bug fix, so it would be much easier to review if they were multiple
commits. You have partially documented your changes, but there are changes
in files not mentioned in your README file. I notice one of the files has a
large amount of commented-out SQL - is this needed?

I'm not clear what the 'Correction bug course id' commit is doing.

The commit 'New branch :' has a non-descriptive commit message. It
appears to be related to exporting to Mahara, but I would prefer to have a
clear description of what it does and how it does it. It also includes at
least one reference to 'mod-referentiel', a number of language strings
which are not in alphabetical order (which makes ongoing maintenance much
more fiddly) and a number of variables / comments / function names in
French (I have nothing against the French language, but as I will need to
maintain any integrated code in the future, it is much more helpful if I
understand clearly what it all means).

"follow_link" icon added as link ... - this looks fairly
straight-forward, but I would prefer redundant code to be removed, rather
than commented out (that is why we have version control software like git).

Few bugs corrections about Mahara stuff ... - again, fairly
straightforward. Maybe should be integrated back into your original Mahara
commit, but not essential, as I can see what is going on there.

Don't lock mahara stuff - this commit only seems to add a comment, that
doesn't really explain what is going on.

Signed-off-by: Jean FRUITET ... - looks like an obvious bug fix, but it
would be helpful to explain exactly what is being fixed.

I know that what I am asking here is probably a large amount of work.
Unfortunately, from my perspective, reviewing the code in the way the
commits are currently organised is also a large amount of work. Unless you
are able to make my job a lot easier, I am afraid I will not be able to
accept these changes. I am sorry if I am being unreasonable in anything I
have written and I hope you understand why I am saying this.
OK.
That's correct.

Reply to this email directly or view it on GitHub:

#3 (comment)

Aucun virus trouvé dans ce message.
Analyse effectuée par AVG - www.avg.fr
Version: 2012.0.2195 / Base de données virale: 2437/5118 - Date:
08/07/2012

Jean FRUITET - St Sébastien / Loire

Reply to this email directly or view it on GitHub:
#3 (comment)

jfruitet · 2012-07-10T07:57:55Z

Thanks you much Davo,
This is the perfect "feuille de route".

I'll stick to it.
Jean.

Le 09/07/2012 20:12, Davo Smith a écrit :

Yes - one commit per new feature / bug fix would be great - one commit per

line would be terrible!

I use the command line version of git and this is how I would proceed.

( git config --global color.ui auto // makes the output a lot prettier )

git add remote davosmith
git://github.com/davosmith/moodle-checklist.git // This adds my
official repo as a source for your repo

git fetch davosmith // Get all the latest code from my repo

git checkout davosmith/master // Check out my master branch

git checkout -b mynewbranch // Create a new branch, off my master
branch, to put your code in (give it a better name than that!)

Copy & paste in the code for the first feature / fix (try to avoid
adding trailing whitespace or adjusting the layout of any of the existing
code)

git diff // to check exactly what you have changed - I can also
recommend gitk (if you have it installed) to check

git add FILENAME // for each file you have changed

(optional) use gitk again, to double-check all the files / changes are
queued up correctly; you can also use 'git status' to check the queued
files as well

git commit -m "Brief description of the feature that is being added" //

If it needs more explanation, leave out the -m "Brief description ...." and
type into the text editor that appears a brief description, a blank line,
then a more detailed explanation.
10. Go back to step 5 until all changes checked in
11. git push origin mynewbranch:mynewbranch // This pushes 'mynewbranch'
to github and creates a new branch there called 'mynewbranch' (substitute
the branch name you used in step 4)
12. Go onto github and put in a pull request from 'mynewbranch'

There are some alternatives to 'git add FILENAME' - you can also use 'git
add *.php' (to add all PHP files in the current folder) or 'git add db' (to
add all files in the db folder + sub folders) or 'git add .' (to add
everything in the current folder and subfolders). You can also, skip the
'git add' altogether and call 'git commit -am "brief description"' if you
know exactly what you have changed, as this adds and commits all the
previously known files (but does not add any new files). Use this last
option with great care (I have often committed changes that I didn't want,

or missed new files with this version).

One thing you might want to do BEFORE step 11, is to do another 'git fetch

davosmith' to see if I have changed anything (unlikely) and then a 'git
rebase davosmith/master' to pull in my latest changes and then re-run your

changes over the top. Do not do this after you have pushed to github, as it
won't work (without a --force parameter).

I hope that helps you out a bit with Git - feel free to ask a question if
you get stuck ( [email protected] )

Davo

On Mon, Jul 9, 2012 at 5:07 PM, Jean FRUITET <
[email protected]

wrote:
Hi Davo,

I had never used GIT.
Surely the way I did that fork is not the righ one.

I shall delete all that fork and redo a brand new one.
I'll try to do a commit per new feature (but surely not one for each
line of code !;>))

New features type 1:
1.1 - outcomes as list of items (affect import / export)
1.2 - outcomes validation made in any Moodle activity pushed to
Checklist+ (affect cron job)

New features type 2:
2.1 - prove of practice attached to each item (affect table structure)
2.2 - Mahara connection (selected links / export data) (uses portfolio)
Please feel free to give me your keen advice, for wich I'll thank you much.

Regards.
Jean.

P.S.

I have presented the CheckList+ to French MoodleMoot 2012.
http://moodlemoot2012.unimes.fr/
http://moodlemoot2012.unimes.fr/course/view.php?id=50

A lot of people interested by that evolution...

JF.

Le 07/07/2012 19:28, Davo Smith a écrit :

I am really sorry that it has taken me such a long time to get around to
reviewing the changes you have made to the checklist module.
I've started looking at the changes this afternoon, but I'm afraid that
I'm struggling to find my way through the changes you have made. For me to
have any chance of reviewing and integrating this, I'm going to have to ask
you to create a cleaned-up version of this, with 1 commit for each new
feature, along with a clear commit message explaining exactly what feature
you are adding.
OK.
I'll try that.

Going through the commits, the first commit 'MODIF JF tags removed'
adjusts the backup to include data tables that have not been added to the
checklist module by that point. It also includes multiple
restore_decode_rule statements, all for decoding the same string
(CHECKLISTEDITBYCHECKLIST), but with different expected results. This is a
clear bug, which you may have fixed in later commits, but it is difficult
for me to locate where such a fix is, if it exists.
The next two commits, both called 'Initial commit for JF fork' appear to
be the same - are there some subtle differences I should be aware of? If
not it would be helpful to include only one of these commits there. These
also, unless I have misunderstood them, introduce more than one new feature
/ bug fix, so it would be much easier to review if they were multiple
commits. You have partially documented your changes, but there are changes
in files not mentioned in your README file. I notice one of the files has a
large amount of commented-out SQL - is this needed?
I'm not clear what the 'Correction bug course id' commit is doing.

The commit 'New branch :' has a non-descriptive commit message. It
appears to be related to exporting to Mahara, but I would prefer to have a
clear description of what it does and how it does it. It also includes at
least one reference to 'mod-referentiel', a number of language strings
which are not in alphabetical order (which makes ongoing maintenance much
more fiddly) and a number of variables / comments / function names in
French (I have nothing against the French language, but as I will need to
maintain any integrated code in the future, it is much more helpful if I
understand clearly what it all means).
"follow_link" icon added as link ... - this looks fairly
straight-forward, but I would prefer redundant code to be removed, rather
than commented out (that is why we have version control software like git).
Few bugs corrections about Mahara stuff ... - again, fairly
straightforward. Maybe should be integrated back into your original Mahara
commit, but not essential, as I can see what is going on there.
Don't lock mahara stuff - this commit only seems to add a comment, that
doesn't really explain what is going on.
Signed-off-by: Jean FRUITET ... - looks like an obvious bug fix, but it
would be helpful to explain exactly what is being fixed.
I know that what I am asking here is probably a large amount of work.
Unfortunately, from my perspective, reviewing the code in the way the
commits are currently organised is also a large amount of work. Unless you
are able to make my job a lot easier, I am afraid I will not be able to
accept these changes. I am sorry if I am being unreasonable in anything I
have written and I hope you understand why I am saying this.
OK.
That's correct.

Reply to this email directly or view it on GitHub:

#3 (comment)

Aucun virus trouvé dans ce message.
Analyse effectuée par AVG - www.avg.fr
Version: 2012.0.2195 / Base de données virale: 2437/5118 - Date:
08/07/2012

Jean FRUITET - St Sébastien / Loire

Reply to this email directly or view it on GitHub:
#3 (comment)

Reply to this email directly or view it on GitHub:
#3 (comment)

Aucun virus trouvé dans ce message.
Analyse effectuée par AVG - www.avg.fr
Version: 2012.0.2195 / Base de données virale: 2437/5121 - Date: 09/07/2012

Jean FRUITET - St Sébastien / Loire

jfruitet · 2012-07-26T15:57:19Z

Hi Davo,

I have did the job for Feature 1 :

CheckList+ Feature 1 : Import / export outcomes files as Item list
[jfruitet]

All the stuff is here :
https://github.com/jfruitet/moodle-checklist/tree/checklistplus

It is a new branch called "checklistplus" based on a today fork of your
https://github.com/davosmith/moodle-checklist

Many files modifed, of course...

Tomorrow I'll push the feature 2.

Regards,

Jean.

Le 09/07/2012 20:12, Davo Smith a écrit :

Yes - one commit per new feature / bug fix would be great - one commit per

line would be terrible!

I use the command line version of git and this is how I would proceed.

( git config --global color.ui auto // makes the output a lot prettier )

git add remote davosmith
git://github.com/davosmith/moodle-checklist.git // This adds my
official repo as a source for your repo
Correct command is
git remote add davosmith git://github.com/davosmith/moodle-checklist.git

git fetch davosmith // Get all the latest code from my repo

git checkout davosmith/master // Check out my master branch

git checkout -b mynewbranch // Create a new branch, off my master
branch, to put your code in (give it a better name than that!)

Copy & paste in the code for the first feature / fix (try to avoid
adding trailing whitespace or adjusting the layout of any of the existing
code)

git diff // to check exactly what you have changed - I can also
recommend gitk (if you have it installed) to check

git add FILENAME // for each file you have changed

(optional) use gitk again, to double-check all the files / changes are
queued up correctly; you can also use 'git status' to check the queued
files as well

git commit -m "Brief description of the feature that is being added" //

If it needs more explanation, leave out the -m "Brief description ...." and
type into the text editor that appears a brief description, a blank line,
then a more detailed explanation.
10. Go back to step 5 until all changes checked in
11. git push origin mynewbranch:mynewbranch // This pushes 'mynewbranch'
to github and creates a new branch there called 'mynewbranch' (substitute
the branch name you used in step 4)
12. Go onto github and put in a pull request from 'mynewbranch'

There are some alternatives to 'git add FILENAME' - you can also use 'git
add *.php' (to add all PHP files in the current folder) or 'git add db' (to
add all files in the db folder + sub folders) or 'git add .' (to add
everything in the current folder and subfolders). You can also, skip the
'git add' altogether and call 'git commit -am "brief description"' if you
know exactly what you have changed, as this adds and commits all the
previously known files (but does not add any new files). Use this last
option with great care (I have often committed changes that I didn't want,

or missed new files with this version).

One thing you might want to do BEFORE step 11, is to do another 'git fetch

davosmith' to see if I have changed anything (unlikely) and then a 'git
rebase davosmith/master' to pull in my latest changes and then re-run your

changes over the top. Do not do this after you have pushed to github, as it
won't work (without a --force parameter).

I hope that helps you out a bit with Git - feel free to ask a question if
you get stuck ( [email protected] )

Davo

On Mon, Jul 9, 2012 at 5:07 PM, Jean FRUITET <
[email protected]

wrote:
Hi Davo,

I had never used GIT.
Surely the way I did that fork is not the righ one.

I shall delete all that fork and redo a brand new one.
I'll try to do a commit per new feature (but surely not one for each
line of code !;>))

New features type 1:
1.1 - outcomes as list of items (affect import / export)
1.2 - outcomes validation made in any Moodle activity pushed to
Checklist+ (affect cron job)

New features type 2:
2.1 - prove of practice attached to each item (affect table structure)
2.2 - Mahara connection (selected links / export data) (uses portfolio)
Please feel free to give me your keen advice, for wich I'll thank you much.

Regards.
Jean.

P.S.

I have presented the CheckList+ to French MoodleMoot 2012.
http://moodlemoot2012.unimes.fr/
http://moodlemoot2012.unimes.fr/course/view.php?id=50

A lot of people interested by that evolution...

JF.

Le 07/07/2012 19:28, Davo Smith a écrit :

I am really sorry that it has taken me such a long time to get around to
reviewing the changes you have made to the checklist module.
I've started looking at the changes this afternoon, but I'm afraid that
I'm struggling to find my way through the changes you have made. For me to
have any chance of reviewing and integrating this, I'm going to have to ask
you to create a cleaned-up version of this, with 1 commit for each new
feature, along with a clear commit message explaining exactly what feature
you are adding.
OK.
I'll try that.

Going through the commits, the first commit 'MODIF JF tags removed'
adjusts the backup to include data tables that have not been added to the
checklist module by that point. It also includes multiple
restore_decode_rule statements, all for decoding the same string
(CHECKLISTEDITBYCHECKLIST), but with different expected results. This is a
clear bug, which you may have fixed in later commits, but it is difficult
for me to locate where such a fix is, if it exists.
The next two commits, both called 'Initial commit for JF fork' appear to
be the same - are there some subtle differences I should be aware of? If
not it would be helpful to include only one of these commits there. These
also, unless I have misunderstood them, introduce more than one new feature
/ bug fix, so it would be much easier to review if they were multiple
commits. You have partially documented your changes, but there are changes
in files not mentioned in your README file. I notice one of the files has a
large amount of commented-out SQL - is this needed?
I'm not clear what the 'Correction bug course id' commit is doing.

The commit 'New branch :' has a non-descriptive commit message. It
appears to be related to exporting to Mahara, but I would prefer to have a
clear description of what it does and how it does it. It also includes at
least one reference to 'mod-referentiel', a number of language strings
which are not in alphabetical order (which makes ongoing maintenance much
more fiddly) and a number of variables / comments / function names in
French (I have nothing against the French language, but as I will need to
maintain any integrated code in the future, it is much more helpful if I
understand clearly what it all means).
"follow_link" icon added as link ... - this looks fairly
straight-forward, but I would prefer redundant code to be removed, rather
than commented out (that is why we have version control software like git).
Few bugs corrections about Mahara stuff ... - again, fairly
straightforward. Maybe should be integrated back into your original Mahara
commit, but not essential, as I can see what is going on there.
Don't lock mahara stuff - this commit only seems to add a comment, that
doesn't really explain what is going on.
Signed-off-by: Jean FRUITET ... - looks like an obvious bug fix, but it
would be helpful to explain exactly what is being fixed.
I know that what I am asking here is probably a large amount of work.
Unfortunately, from my perspective, reviewing the code in the way the
commits are currently organised is also a large amount of work. Unless you
are able to make my job a lot easier, I am afraid I will not be able to
accept these changes. I am sorry if I am being unreasonable in anything I
have written and I hope you understand why I am saying this.
OK.
That's correct.

Reply to this email directly or view it on GitHub:

#3 (comment)

Aucun virus trouvé dans ce message.
Analyse effectuée par AVG - www.avg.fr
Version: 2012.0.2195 / Base de données virale: 2437/5118 - Date:
08/07/2012

Jean FRUITET - St Sébastien / Loire

Reply to this email directly or view it on GitHub:
#3 (comment)

Reply to this email directly or view it on GitHub:
#3 (comment)

Aucun virus trouvé dans ce message.
Analyse effectuée par AVG - www.avg.fr
Version: 2012.0.2195 / Base de données virale: 2437/5121 - Date: 09/07/2012

Jean FRUITET - Université de Nantes - France

davosmith · 2012-07-26T16:09:47Z

Thanks for that, I'll try to look at it as soon as possible, but I'm going
to be on holiday for the next week, so will be a little while.
On Jul 26, 2012 4:57 PM, "Jean FRUITET" <
[email protected]>
wrote:

Hi Davo,

I have did the job for Feature 1 :

CheckList+ Feature 1 : Import / export outcomes files as Item list
[jfruitet]

All the stuff is here :
https://github.com/jfruitet/moodle-checklist/tree/checklistplus

It is a new branch called "checklistplus" based on a today fork of your
https://github.com/davosmith/moodle-checklist

Many files modifed, of course...

Tomorrow I'll push the feature 2.

Regards,

Jean.

Le 09/07/2012 20:12, Davo Smith a écrit :

Yes - one commit per new feature / bug fix would be great - one commit
per

line would be terrible!

I use the command line version of git and this is how I would proceed.

( git config --global color.ui auto // makes the output a lot prettier )

git add remote davosmith
git://github.com/davosmith/moodle-checklist.git // This adds my
official repo as a source for your repo
Correct command is
git remote add davosmith git://github.com/davosmith/moodle-checklist.git

git fetch davosmith // Get all the latest code from my repo

git checkout davosmith/master // Check out my master branch

git checkout -b mynewbranch // Create a new branch, off my master
branch, to put your code in (give it a better name than that!)

Copy & paste in the code for the first feature / fix (try to avoid
adding trailing whitespace or adjusting the layout of any of the existing
code)

git diff // to check exactly what you have changed - I can also
recommend gitk (if you have it installed) to check

git add FILENAME // for each file you have changed

(optional) use gitk again, to double-check all the files / changes are
queued up correctly; you can also use 'git status' to check the queued
files as well

git commit -m "Brief description of the feature that is being added"
//

If it needs more explanation, leave out the -m "Brief description ...."
and
type into the text editor that appears a brief description, a blank line,
then a more detailed explanation.
10. Go back to step 5 until all changes checked in
11. git push origin mynewbranch:mynewbranch // This pushes 'mynewbranch'
to github and creates a new branch there called 'mynewbranch' (substitute
the branch name you used in step 4)
12. Go onto github and put in a pull request from 'mynewbranch'

There are some alternatives to 'git add FILENAME' - you can also use 'git
add *.php' (to add all PHP files in the current folder) or 'git add db'
(to
add all files in the db folder + sub folders) or 'git add .' (to add
everything in the current folder and subfolders). You can also, skip the
'git add' altogether and call 'git commit -am "brief description"' if you
know exactly what you have changed, as this adds and commits all the
previously known files (but does not add any new files). Use this last
option with great care (I have often committed changes that I didn't
want,

or missed new files with this version).

One thing you might want to do BEFORE step 11, is to do another 'git
fetch

davosmith' to see if I have changed anything (unlikely) and then a 'git
rebase davosmith/master' to pull in my latest changes and then re-run
your

changes over the top. Do not do this after you have pushed to github, as
it
won't work (without a --force parameter).

I hope that helps you out a bit with Git - feel free to ask a question if
you get stuck ( [email protected] )

Davo

On Mon, Jul 9, 2012 at 5:07 PM, Jean FRUITET <
[email protected]

wrote:
Hi Davo,

I had never used GIT.
Surely the way I did that fork is not the righ one.

I shall delete all that fork and redo a brand new one.
I'll try to do a commit per new feature (but surely not one for each
line of code !;>))

New features type 1:
1.1 - outcomes as list of items (affect import / export)
1.2 - outcomes validation made in any Moodle activity pushed to
Checklist+ (affect cron job)

New features type 2:
2.1 - prove of practice attached to each item (affect table structure)
2.2 - Mahara connection (selected links / export data) (uses portfolio)
Please feel free to give me your keen advice, for wich I'll thank you
much.

Regards.
Jean.

P.S.

I have presented the CheckList+ to French MoodleMoot 2012.
http://moodlemoot2012.unimes.fr/
http://moodlemoot2012.unimes.fr/course/view.php?id=50

A lot of people interested by that evolution...

JF.

Le 07/07/2012 19:28, Davo Smith a écrit :

I am really sorry that it has taken me such a long time to get around
to
reviewing the changes you have made to the checklist module.
I've started looking at the changes this afternoon, but I'm afraid that
I'm struggling to find my way through the changes you have made. For me
to
have any chance of reviewing and integrating this, I'm going to have to
ask
you to create a cleaned-up version of this, with 1 commit for each new
feature, along with a clear commit message explaining exactly what
feature
you are adding.
OK.
I'll try that.

Going through the commits, the first commit 'MODIF JF tags removed'
adjusts the backup to include data tables that have not been added to
the
checklist module by that point. It also includes multiple
restore_decode_rule statements, all for decoding the same string
(CHECKLISTEDITBYCHECKLIST), but with different expected results. This
is a
clear bug, which you may have fixed in later commits, but it is
difficult
for me to locate where such a fix is, if it exists.
The next two commits, both called 'Initial commit for JF fork' appear
to
be the same - are there some subtle differences I should be aware of? If
not it would be helpful to include only one of these commits there.
These
also, unless I have misunderstood them, introduce more than one new
feature
/ bug fix, so it would be much easier to review if they were multiple
commits. You have partially documented your changes, but there are
changes
in files not mentioned in your README file. I notice one of the files
has a
large amount of commented-out SQL - is this needed?
I'm not clear what the 'Correction bug course id' commit is doing.

The commit 'New branch :' has a non-descriptive commit message. It
appears to be related to exporting to Mahara, but I would prefer to
have a
clear description of what it does and how it does it. It also includes
at
least one reference to 'mod-referentiel', a number of language strings
which are not in alphabetical order (which makes ongoing maintenance
much
more fiddly) and a number of variables / comments / function names in
French (I have nothing against the French language, but as I will need
to
maintain any integrated code in the future, it is much more helpful if I
understand clearly what it all means).
"follow_link" icon added as link ... - this looks fairly
straight-forward, but I would prefer redundant code to be removed,
rather
than commented out (that is why we have version control software like
git).
Few bugs corrections about Mahara stuff ... - again, fairly
straightforward. Maybe should be integrated back into your original
Mahara
commit, but not essential, as I can see what is going on there.
Don't lock mahara stuff - this commit only seems to add a comment, that
doesn't really explain what is going on.
Signed-off-by: Jean FRUITET ... - looks like an obvious bug fix, but it
would be helpful to explain exactly what is being fixed.
I know that what I am asking here is probably a large amount of work.
Unfortunately, from my perspective, reviewing the code in the way the
commits are currently organised is also a large amount of work. Unless
you
are able to make my job a lot easier, I am afraid I will not be able to
accept these changes. I am sorry if I am being unreasonable in anything
I
have written and I hope you understand why I am saying this.
OK.
That's correct.

Reply to this email directly or view it on GitHub:

#3 (comment)

Aucun virus trouvé dans ce message.
Analyse effectuée par AVG - www.avg.fr
Version: 2012.0.2195 / Base de données virale: 2437/5118 - Date:
08/07/2012

Jean FRUITET - St Sébastien / Loire

Reply to this email directly or view it on GitHub:

#3 (comment)

Reply to this email directly or view it on GitHub:

#3 (comment)

Aucun virus trouvé dans ce message.
Analyse effectuée par AVG - www.avg.fr
Version: 2012.0.2195 / Base de données virale: 2437/5121 - Date:
09/07/2012

Jean FRUITET - Université de Nantes - France

Reply to this email directly or view it on GitHub:
#3 (comment)

jfruitet added 4 commits April 11, 2012 14:47

MODIF JF tags removed

e3807a9

Initial commit for JF fork

e118c1e

Signed-off-by: Jean FRUITET <[email protected]>

Initial commit for JF fork

0223d1a

Signed-off-by: Jean FRUITET <[email protected]>

Correction bug course id

caadaaf

Signed-off-by: Jean FRUITET <[email protected]>

jfruitet added 5 commits April 13, 2012 15:54

New branch :

f2b83cd

upload CheckList as HTML / LEAP2 to Mahara downlod View from mahara as prove of practice Nouvelle branche : déposer une CheckList vers Mahara Télécharger une vue mahara comme preuve de pratique

"follow_link" icon added as link to the activity which validated that…

e385264

… item (outcome). table checklist_comment, scrip cron_outcomes.php

Few bugs corrections about Mahara stuff. Mahara pages linked to check…

9667860

…list item are not yet locked. Signed-off-by: Jean FRUITET <[email protected]>

Don't lock mahara stuff

9a76eea

Signed-off-by: Jean FRUITET <[email protected]>

Signed-off-by: Jean FRUITET <[email protected]>

2a16514

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Hi Davo, checklist fork ready #3

Hi Davo, checklist fork ready #3

jfruitet commented Apr 11, 2012

davosmith commented Apr 11, 2012

davosmith commented Jul 7, 2012

jfruitet commented Jul 9, 2012

davosmith commented Jul 9, 2012

jfruitet commented Jul 10, 2012

jfruitet commented Jul 26, 2012

davosmith commented Jul 26, 2012

Hi Davo, checklist fork ready #3

Are you sure you want to change the base?

Hi Davo, checklist fork ready #3

Conversation

jfruitet commented Apr 11, 2012

davosmith commented Apr 11, 2012

davosmith commented Jul 7, 2012

jfruitet commented Jul 9, 2012

davosmith commented Jul 9, 2012

jfruitet commented Jul 10, 2012

jfruitet commented Jul 26, 2012

davosmith commented Jul 26, 2012