Hi Danielle,
thank you for reporting this.
Andreas

I'm not really sure this is a bug - its definitely not been implemented before but I'm happy to put this into master.

Hi Andreas,
i've found some whitespaces and code formatting issues which are minor but i'm short on time today.

also, please send this issue up for peer-review , it helps that there are more eyes looking at the code going into core

(still thinking its going into master only tho, will look at this next week)

Hi Aparup,
I do not understand, what I have to do. How can I send this issue up for peer-review? What is this meaning?
Andreas

Hi Andreas,
i think the work done is great but i think its actually a new function right? not really a bug - so its better that this goes only into master (instead of changing things in STABLE versions).

besides that i noticed this issue wasn't peer-reviewed. we've got a peer-review process described @ http://docs.moodle.org/dev/Process - basically after development it helps to get the issue peer reviewed. so in the end at least 3 eyes have gone through the code (1:developer ,2: peer-reviewer, 3: integrator)

Ah, i'm guessing you probably can't see the 'peer-review' button ? It might be under the workflow menu for you among the 'send for integration group of buttons. We really should have images/docs for this in the process doc.

This way we can reduce bugs/regressions and keep moodle quality high

ps: i can fix this up on friday and get it integrated soon - i ran out of time. Just wanted to let you know about 'peer-review' stage in the workflow - cheers!

Hi Aparup,
sorry, my english is not so good.
If I understand you right I have to go to a button "peer-review". But I can't see such button. I did never need this before . Where can I do this?
Andreas

Hi Andreas, please check with Helen or Michael (or MD?) - you should be able to see the peer-review button as a developer.

edit: ah! the 'peer-review' option is only visible when the issue is in development. i'll push it along now to in-development so that you can see the 'peer-review'.

Andreas, its now in 'development in progress', you should be able to request 'peer-review' now. i can see it. i'll leave that bit to you to get familiar with

Me? Was it a distraction?

Ok, now I start peer review. But I do not know who I should select for "Peer reviewer".
Andreas

Gal that its working for you Andreas

it will now appear in a list for HQ to review (its optional to assign a peer-reviewer)

Hi Aparup,

there is still another question. Why do you say it is not a bug? Anybody who use an editor is expecting this function. I think it is a very important thing and should not wait until 2.3 is coming out. At least for 2.2 it should be go in.

Best regards Andreas

Hi,
I would like to know whether there are some things I can/have to do so that these commits will be integrated.
Best regards
Andreas

Hi Andreas, i'm sending this up for integration now.

I'm sure its expected to be able to put up pictures in "Page after submit". I just meant it wasn't implemented at all, its great it will be in there soon!

The pre-checker has found a few coding style problems:

<description>

This sections shows the coding style problems detected in the code by phpcs

</description>

<mess>

<problem file="mod/feedback/lib.php" linefrom="736" lineto="736" method="" class="" package="" api="" ruleset="Squiz" rule="Commenting.DocCommentAlignment.SpaceBeforeTag" url="http://docs.moodle.org/dev/Coding_style" weight="5">

<message>Expected 1 space between asterisk and tag; 2 found</message>

<description/>

<code/>

</problem>

<problem file="mod/feedback/mod_form.php" linefrom="172" lineto="172" method="" class="" package="" api="" ruleset="Squiz" rule="WhiteSpace.SuperfluousWhitespace.EndLine" url="http://docs.moodle.org/dev/Coding_style" weight="5">

<message>Whitespace found at end of line</message>

<description/>

<code/>

</problem>

<problem file="mod/feedback/mod_form.php" linefrom="178" lineto="178" method="" class="" package="" api="" ruleset="Squiz" rule="WhiteSpace.SuperfluousWhitespace.EndLine" url="http://docs.moodle.org/dev/Coding_style" weight="5">

<message>Whitespace found at end of line</message>

<description/>

<code/>

</problem>

<problem file="mod/feedback/mod_form.php" linefrom="189" lineto="189" method="" class="" package="" api="" ruleset="moodle" rule="Files.LineLength.TooLong" url="http://docs.moodle.org/dev/Coding_style" weight="1">

<message>

Line exceeds 132 characters; contains 138 characters

</message>

<description/>

<code/>

</problem>

Hi Dan,

I have fixed the shown errors at master and 2.2.
There still are some warnings but I think it makes no sense to shorten the related lines.

Best regards
Andreas

Hi Andreas,

I notice that you are using itemid for this with the feedback id. Its not necessary to do this as the context of the filarea will link the item back to the feedback id in the main mod_feedback table.

It'll be clearer if the itemid is not used as in this case its not necessary (the files are shared throughout the the whole feeback activity). So could you switch to not using the itemid. See the workfshop module for an example of that - with instructauthors.

Thanks!
dan

Hi Dan,

ok, now I understand it better a bit .
I have fixed these things.

Best regards
Andreas

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Thanks, that has been integrated now

This looks good

Test passed.

The code here has been spread to upstream moodle repositories and mirrors for anyone to use .

Closing, have a good weekend!