You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@ofbiz.apache.org by "gil.portenseigne" <gi...@nereide.fr> on 2023/02/06 08:18:00 UTC

Re: Codenarc integration process

Hello Daniel,

Thanks again for the review you did, could we add a small description
when UNSURE or WORK_NEEDED is set in the review table ?

Or will it be best to use github comments in pull request ?

I'm curious about the reason, and would like to help solve them.

I will try to advance this week in the review process.

Regards, 

Gil

On 28/01/23 08:46, Daniel Watford wrote:
> Turns out I was able to import the list of files into Excel and copy and
> paste the table from Excel to Confluence.
> 
> On Sat, 28 Jan 2023 at 08:37, Daniel Watford <da...@foomoo.co.uk> wrote:
> 
> > Hi Gil,
> >
> > I don't think a checklist is quite enough, assuming we want to track the
> > status of each file reviewed.
> >
> > From the review approach section:
> >
> >
> >    - If in the reviewers opinion a file change will not change OFBiz
> >    behaviour in any way they should mark the corresponding entry in the table
> >    below as PASSED.
> >    - If the reviewer identifies an issue with a changed file, then they
> >    should add a comment in the PR on GitHub AND mark the corresponding entry
> >    in the table below as WORK NEEDED.
> >    - If the reviewer is unsure how to classify a changed file they should
> >    mark the corresponding entry in the table below as UNSURE.
> >    - In each of the above cases, the reviewer should add their name
> >    against the entry in the table below.
> >
> > The checklist doesn't give us the opportunity to see what files need some
> > additional help.
> >
> > I'm sure there must be some way of getting Confluence to produce a table
> > from a list - I just don't seem to have found it yet! I'll play around with
> > Confluence a bit more.
> >
> > But as mentioned before, perhaps I am making too much out of tracking this
> > review.
> >
> > Thanks,
> >
> > Dan.
> >
> >
> > On Fri, 27 Jan 2023 at 17:05, gil.portenseigne <
> > gil.portenseigne@nereide.fr> wrote:
> >
> >> I got to leave, but i generated in confluence a list of check, is that
> >> good enough ?
> >>
> >> Gil
> >> On 27/01/23 05:41, gil.portenseigne wrote:
> >> > Hello, indeed, that will generate much spam, i did some before reading
> >> > your answer.
> >> >
> >> > I'll have a look for conluence.
> >> >
> >> > Gil
> >> >
> >> >
> >> > On 27/01/23 04:14, Daniel Watford wrote:
> >> > > Hi Gill and Jacques,
> >> > >
> >> > > I don't think we should add comments to the PR to track the files
> >> that we
> >> > > have reviewed as I think each comment will appear separately in the
> >> PR's
> >> > > conversation view.
> >> > >
> >> > > However, with such a large PR where we hope to get several reviewers
> >> > > involved I think we do need a mechanism to track reviewed files.
> >> > >
> >> > > I created a page here - Codenarc integration review tracker - OFBiz
> >> Project
> >> > > Open Wiki - Apache Software Foundation
> >> > > <
> >> https://cwiki.apache.org/confluence/display/OFBIZ/Codenarc+integration+review+tracker
> >> >
> >> > > -
> >> > > suggesting an approach.
> >> > >
> >> > > If the approach is acceptable then all reviewers should be able to
> >> update
> >> > > the page as we go.
> >> > >
> >> > > I'm stuck with finding a nice way to generate a table listing all the
> >> > > changed files and the review status of each file. I have included the
> >> > > commands to produce the list of files and shown some examples of how
> >> to add
> >> > > a header, but my attempts to turn that into something useful on a
> >> > > confluence page have not been fruitful.
> >> > >
> >> > > So two questions.
> >> > > - Is it worth coming up with a page/table to track this PR or am I
> >> just
> >> > > creating unnecessary admin work when we could use comments in the PR?
> >> > > - Can anyone create a table in Confluence that we could use to track
> >> the
> >> > > review effort?
> >> > >
> >> > > Thanks,
> >> > >
> >> > > Dan.
> >> > >
> >> > >
> >> > > On Fri, 27 Jan 2023 at 15:27, gil.portenseigne <
> >> gil.portenseigne@nereide.fr>
> >> > > wrote:
> >> > >
> >> > > > Oops, i did a fixup commit with push force that remove all comments
> >> in
> >> > > > the pull request... Will not do that again.
> >> > > >
> >> > > > I fixed the detected typo.
> >> > > >
> >> > > > gil
> >> > > > On 27/01/23 02:56, Jacques Le Roux wrote:
> >> > > > > Ah OK, sounds better indeed
> >> > > > >
> >> > > > > Le 27/01/2023 à 14:06, gil.portenseigne a écrit :
> >> > > > > > The idea is not to modify the files, but to add a comment into
> >> the pull
> >> > > > > > request. Those allowing each reviewer to check the viewed
> >> checkbox if a
> >> > > > > > comment is present, to collapse already reviewed files.
> >> > > > > >
> >> > > > > > So no need further action, apart the real code modification
> >> request,
> >> > > > > > when commiting the code.
> >> > > > > >
> >> > > > > > On 27/01/23 12:00, Jacques Le Roux wrote:
> >> > > > > > > Hi Gil, Daniel,
> >> > > > > > >
> >> > > > > > > I agree Gil, I just tried before seeing your message and came
> >> to the
> >> > > > same conclusion.
> >> > > > > > >
> >> > > > > > > With a comment at top we would need to remove it later,
> >> right? Could
> >> > > > be easy if it's the same unique words in every file.
> >> > > > > > >
> >> > > > > > > Jacques
> >> > > > > > >
> >> > > > > > > Le 27/01/2023 à 10:41, gil.portenseigne a écrit :
> >> > > > > > > > Hi Daniel, Jacques,
> >> > > > > > > >
> >> > > > > > > > I wonders the same, the "Review changes" do not seems to
> >> concern
> >> > > > one
> >> > > > > > > > file but the whole pull request, there is a review
> >> checkbox, but it
> >> > > > > > > > seems to be personal, i checked the first one
> >> > > > > > > > (AcctgAdminServices.groovy) for testing purpose.
> >> > > > > > > >
> >> > > > > > > > What we could do is to add a comment at the start of each
> >> file, to
> >> > > > let
> >> > > > > > > > others know that review job has been done.
> >> > > > > > > >
> >> > > > > > > > WDYT ?
> >> > > > > > > >
> >> > > > > > > > Gil
> >> > > > > > > >
> >> > > > > > > >
> >> > > > > > > > On 26/01/23 07:48, Jacques Le Roux wrote:
> >> > > > > > > > > Hi Daniel,
> >> > > > > > > > >
> >> > > > > > > > > In "Files changed" tab*, when you select a file, the
> >> "Review
> >> > > > changes" button allows you to comment, approve or request changes
> >> on this
> >> > > > file.
> >> > > > > > > > > I guess "approve" is what you are looking for?
> >> > > > > > > > >
> >> > > > > > > > > *
> >> https://github.com/apache/ofbiz-framework/pull/517/files
> >> > > > > > > > >
> >> > > > > > > > > Le 26/01/2023 à 17:26, Daniel Watford a écrit :
> >> > > > > > > > > > Does anyone know of a way in a GitHub PR that a
> >> reviewer can
> >> > > > mark an
> >> > > > > > > > > > individual file as reviewed-and-passed so that other
> >> reviewers
> >> > > > can skip
> >> > > > > > > > > > that file?
> >> > > >
> >> > >
> >> > >
> >> > > --
> >> > > Daniel Watford
> >>
> >>
> >>
> >
> > --
> > Daniel Watford
> >
> 
> 
> -- 
> Daniel Watford

Re: Codenarc integration process

Posted by Daniel Watford <da...@foomoo.co.uk>.
No, we should be able to create multiple reviews within a single PR - or
perhaps add a comment to an existing review.


On Tue, 7 Feb 2023 at 17:44, Jacques Le Roux <ja...@les7arts.com>
wrote:

> Le 07/02/2023 à 16:53, gil.portenseigne a écrit :
> > yes ongoing review are private, you need to finish it to let other see
> > your comments and suggestions.
>
> Does it mean that you need to review ALL before others see what you
> commented 😮 ?
>
> Thanks
>
> Jacques
>
>

-- 
Daniel Watford

Re: Codenarc integration process

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 07/02/2023 à 16:53, gil.portenseigne a écrit :
> yes ongoing review are private, you need to finish it to let other see
> your comments and suggestions.

Does it mean that you need to review ALL before others see what you commented 😮 ?

Thanks

Jacques


Re: Codenarc integration process

Posted by "gil.portenseigne" <gi...@nereide.fr>.
Hello Dan,

Inline

On 07/02/23 03:21, Daniel Watford wrote:
> Hi Gil,
> 
> There is a comment 'Can requestIdMap be removed?' against the
> StatsSinceStart.groovy file in the PR. This comment is part of a review.
> 
> JobDetails.groovy was marked as UNSURE as it concerned a possible feature
> of Groovy that I wasn't aware of and couldn't find appropriate
> documentation for. The comment against JobDetails.groovy would hopefully
> get another reviewer to take a look and provide a bit more info. This gives
> me a chance to learn something! :)
> 
> The comments in the PR are part of my ongoing review. Since the review has
> not been submitted yet, perhaps it's not visible to you?   If that's the
> case then we'll need to submit multiple reviews as we go.

yes ongoing review are private, you need to finish it to let other see
your comments and suggestions.

I'll be glad to answer you as soon as possible :)

Gil
> 
> Please could you let me know if my review within PR 517 is visible to you.
> 
> Thanks,
> 
> Dan.
> 
> 
> On Mon, 6 Feb 2023 at 12:35, gil.portenseigne <gi...@nereide.fr>
> wrote:
> 
> > Indeed, i agree it is better in PR (sorry again for missing it), but i
> > did not find comments about the StatsSinceStart.groovy and other files
> > you noted as unpassed.
> >
> > Did I miss something ?
> >
> > Gil
> > On 06/02/23 09:10, Daniel Watford wrote:
> > > Hi Gil,
> > >
> > > The Review Approach section of the tracking document says that comments
> > > should be added to the PR when marking a file as WORK_NEEDED. In my case
> > I
> > > added review comments to the PR.
> > >
> > > However I didn't specify how a reviewer should communicate why they were
> > > UNSURE about any particular file. I had added a comment to the PR, but
> > > depending on what the issue is, adding a note to the table might be
> > > appropriate.
> > >
> > > I think comments in PRs are a reasonable approach since they will keep
> > the
> > > comment alongside the code that needs to be worked on or discussed. I
> > would
> > > rather not duplicate comments from the PR in the tracking document.
> > >
> > > Thanks,
> > >
> > > Dan.
> > >
> > > On Mon, 6 Feb 2023 at 08:18, gil.portenseigne <
> > gil.portenseigne@nereide.fr>
> > > wrote:
> > >
> > > > Hello Daniel,
> > > >
> > > > Thanks again for the review you did, could we add a small description
> > > > when UNSURE or WORK_NEEDED is set in the review table ?
> > > >
> > > > Or will it be best to use github comments in pull request ?
> > > >
> > > > I'm curious about the reason, and would like to help solve them.
> > > >
> > > > I will try to advance this week in the review process.
> > > >
> > > > Regards,
> > > >
> > > > Gil
> > > >
> > > > On 28/01/23 08:46, Daniel Watford wrote:
> > > > > Turns out I was able to import the list of files into Excel and copy
> > and
> > > > > paste the table from Excel to Confluence.
> > > > >
> > > > > On Sat, 28 Jan 2023 at 08:37, Daniel Watford <da...@foomoo.co.uk>
> > wrote:
> > > > >
> > > > > > Hi Gil,
> > > > > >
> > > > > > I don't think a checklist is quite enough, assuming we want to
> > track
> > > > the
> > > > > > status of each file reviewed.
> > > > > >
> > > > > > From the review approach section:
> > > > > >
> > > > > >
> > > > > >    - If in the reviewers opinion a file change will not change
> > OFBiz
> > > > > >    behaviour in any way they should mark the corresponding entry in
> > > > the table
> > > > > >    below as PASSED.
> > > > > >    - If the reviewer identifies an issue with a changed file, then
> > they
> > > > > >    should add a comment in the PR on GitHub AND mark the
> > corresponding
> > > > entry
> > > > > >    in the table below as WORK NEEDED.
> > > > > >    - If the reviewer is unsure how to classify a changed file they
> > > > should
> > > > > >    mark the corresponding entry in the table below as UNSURE.
> > > > > >    - In each of the above cases, the reviewer should add their name
> > > > > >    against the entry in the table below.
> > > > > >
> > > > > > The checklist doesn't give us the opportunity to see what files
> > need
> > > > some
> > > > > > additional help.
> > > > > >
> > > > > > I'm sure there must be some way of getting Confluence to produce a
> > > > table
> > > > > > from a list - I just don't seem to have found it yet! I'll play
> > around
> > > > with
> > > > > > Confluence a bit more.
> > > > > >
> > > > > > But as mentioned before, perhaps I am making too much out of
> > tracking
> > > > this
> > > > > > review.
> > > > > >
> > > > > > Thanks,
> > > > > >
> > > > > > Dan.
> > > > > >
> > > > > >
> > > > > > On Fri, 27 Jan 2023 at 17:05, gil.portenseigne <
> > > > > > gil.portenseigne@nereide.fr> wrote:
> > > > > >
> > > > > >> I got to leave, but i generated in confluence a list of check, is
> > that
> > > > > >> good enough ?
> > > > > >>
> > > > > >> Gil
> > > > > >> On 27/01/23 05:41, gil.portenseigne wrote:
> > > > > >> > Hello, indeed, that will generate much spam, i did some before
> > > > reading
> > > > > >> > your answer.
> > > > > >> >
> > > > > >> > I'll have a look for conluence.
> > > > > >> >
> > > > > >> > Gil
> > > > > >> >
> > > > > >> >
> > > > > >> > On 27/01/23 04:14, Daniel Watford wrote:
> > > > > >> > > Hi Gill and Jacques,
> > > > > >> > >
> > > > > >> > > I don't think we should add comments to the PR to track the
> > files
> > > > > >> that we
> > > > > >> > > have reviewed as I think each comment will appear separately
> > in
> > > > the
> > > > > >> PR's
> > > > > >> > > conversation view.
> > > > > >> > >
> > > > > >> > > However, with such a large PR where we hope to get several
> > > > reviewers
> > > > > >> > > involved I think we do need a mechanism to track reviewed
> > files.
> > > > > >> > >
> > > > > >> > > I created a page here - Codenarc integration review tracker -
> > > > OFBiz
> > > > > >> Project
> > > > > >> > > Open Wiki - Apache Software Foundation
> > > > > >> > > <
> > > > > >>
> > > >
> > https://cwiki.apache.org/confluence/display/OFBIZ/Codenarc+integration+review+tracker
> > > > > >> >
> > > > > >> > > -
> > > > > >> > > suggesting an approach.
> > > > > >> > >
> > > > > >> > > If the approach is acceptable then all reviewers should be
> > able to
> > > > > >> update
> > > > > >> > > the page as we go.
> > > > > >> > >
> > > > > >> > > I'm stuck with finding a nice way to generate a table listing
> > all
> > > > the
> > > > > >> > > changed files and the review status of each file. I have
> > included
> > > > the
> > > > > >> > > commands to produce the list of files and shown some examples
> > of
> > > > how
> > > > > >> to add
> > > > > >> > > a header, but my attempts to turn that into something useful
> > on a
> > > > > >> > > confluence page have not been fruitful.
> > > > > >> > >
> > > > > >> > > So two questions.
> > > > > >> > > - Is it worth coming up with a page/table to track this PR or
> > am I
> > > > > >> just
> > > > > >> > > creating unnecessary admin work when we could use comments in
> > the
> > > > PR?
> > > > > >> > > - Can anyone create a table in Confluence that we could use to
> > > > track
> > > > > >> the
> > > > > >> > > review effort?
> > > > > >> > >
> > > > > >> > > Thanks,
> > > > > >> > >
> > > > > >> > > Dan.
> > > > > >> > >
> > > > > >> > >
> > > > > >> > > On Fri, 27 Jan 2023 at 15:27, gil.portenseigne <
> > > > > >> gil.portenseigne@nereide.fr>
> > > > > >> > > wrote:
> > > > > >> > >
> > > > > >> > > > Oops, i did a fixup commit with push force that remove all
> > > > comments
> > > > > >> in
> > > > > >> > > > the pull request... Will not do that again.
> > > > > >> > > >
> > > > > >> > > > I fixed the detected typo.
> > > > > >> > > >
> > > > > >> > > > gil
> > > > > >> > > > On 27/01/23 02:56, Jacques Le Roux wrote:
> > > > > >> > > > > Ah OK, sounds better indeed
> > > > > >> > > > >
> > > > > >> > > > > Le 27/01/2023 à 14:06, gil.portenseigne a écrit :
> > > > > >> > > > > > The idea is not to modify the files, but to add a
> > comment
> > > > into
> > > > > >> the pull
> > > > > >> > > > > > request. Those allowing each reviewer to check the
> > viewed
> > > > > >> checkbox if a
> > > > > >> > > > > > comment is present, to collapse already reviewed files.
> > > > > >> > > > > >
> > > > > >> > > > > > So no need further action, apart the real code
> > modification
> > > > > >> request,
> > > > > >> > > > > > when commiting the code.
> > > > > >> > > > > >
> > > > > >> > > > > > On 27/01/23 12:00, Jacques Le Roux wrote:
> > > > > >> > > > > > > Hi Gil, Daniel,
> > > > > >> > > > > > >
> > > > > >> > > > > > > I agree Gil, I just tried before seeing your message
> > and
> > > > came
> > > > > >> to the
> > > > > >> > > > same conclusion.
> > > > > >> > > > > > >
> > > > > >> > > > > > > With a comment at top we would need to remove it
> > later,
> > > > > >> right? Could
> > > > > >> > > > be easy if it's the same unique words in every file.
> > > > > >> > > > > > >
> > > > > >> > > > > > > Jacques
> > > > > >> > > > > > >
> > > > > >> > > > > > > Le 27/01/2023 à 10:41, gil.portenseigne a écrit :
> > > > > >> > > > > > > > Hi Daniel, Jacques,
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > I wonders the same, the "Review changes" do not
> > seems to
> > > > > >> concern
> > > > > >> > > > one
> > > > > >> > > > > > > > file but the whole pull request, there is a review
> > > > > >> checkbox, but it
> > > > > >> > > > > > > > seems to be personal, i checked the first one
> > > > > >> > > > > > > > (AcctgAdminServices.groovy) for testing purpose.
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > What we could do is to add a comment at the start of
> > > > each
> > > > > >> file, to
> > > > > >> > > > let
> > > > > >> > > > > > > > others know that review job has been done.
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > WDYT ?
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > Gil
> > > > > >> > > > > > > >
> > > > > >> > > > > > > >
> > > > > >> > > > > > > > On 26/01/23 07:48, Jacques Le Roux wrote:
> > > > > >> > > > > > > > > Hi Daniel,
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > In "Files changed" tab*, when you select a file,
> > the
> > > > > >> "Review
> > > > > >> > > > changes" button allows you to comment, approve or request
> > > > changes
> > > > > >> on this
> > > > > >> > > > file.
> > > > > >> > > > > > > > > I guess "approve" is what you are looking for?
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > *
> > > > > >> https://github.com/apache/ofbiz-framework/pull/517/files
> > > > > >> > > > > > > > >
> > > > > >> > > > > > > > > Le 26/01/2023 à 17:26, Daniel Watford a écrit :
> > > > > >> > > > > > > > > > Does anyone know of a way in a GitHub PR that a
> > > > > >> reviewer can
> > > > > >> > > > mark an
> > > > > >> > > > > > > > > > individual file as reviewed-and-passed so that
> > other
> > > > > >> reviewers
> > > > > >> > > > can skip
> > > > > >> > > > > > > > > > that file?
> > > > > >> > > >
> > > > > >> > >
> > > > > >> > >
> > > > > >> > > --
> > > > > >> > > Daniel Watford
> > > > > >>
> > > > > >>
> > > > > >>
> > > > > >
> > > > > > --
> > > > > > Daniel Watford
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > Daniel Watford
> > > >
> > >
> > >
> > > --
> > > Daniel Watford
> >
> 
> 
> -- 
> Daniel Watford

Re: Codenarc integration process

Posted by Daniel Watford <da...@foomoo.co.uk>.
Hi Gil,

There is a comment 'Can requestIdMap be removed?' against the
StatsSinceStart.groovy file in the PR. This comment is part of a review.

JobDetails.groovy was marked as UNSURE as it concerned a possible feature
of Groovy that I wasn't aware of and couldn't find appropriate
documentation for. The comment against JobDetails.groovy would hopefully
get another reviewer to take a look and provide a bit more info. This gives
me a chance to learn something! :)

The comments in the PR are part of my ongoing review. Since the review has
not been submitted yet, perhaps it's not visible to you?   If that's the
case then we'll need to submit multiple reviews as we go.

Please could you let me know if my review within PR 517 is visible to you.

Thanks,

Dan.


On Mon, 6 Feb 2023 at 12:35, gil.portenseigne <gi...@nereide.fr>
wrote:

> Indeed, i agree it is better in PR (sorry again for missing it), but i
> did not find comments about the StatsSinceStart.groovy and other files
> you noted as unpassed.
>
> Did I miss something ?
>
> Gil
> On 06/02/23 09:10, Daniel Watford wrote:
> > Hi Gil,
> >
> > The Review Approach section of the tracking document says that comments
> > should be added to the PR when marking a file as WORK_NEEDED. In my case
> I
> > added review comments to the PR.
> >
> > However I didn't specify how a reviewer should communicate why they were
> > UNSURE about any particular file. I had added a comment to the PR, but
> > depending on what the issue is, adding a note to the table might be
> > appropriate.
> >
> > I think comments in PRs are a reasonable approach since they will keep
> the
> > comment alongside the code that needs to be worked on or discussed. I
> would
> > rather not duplicate comments from the PR in the tracking document.
> >
> > Thanks,
> >
> > Dan.
> >
> > On Mon, 6 Feb 2023 at 08:18, gil.portenseigne <
> gil.portenseigne@nereide.fr>
> > wrote:
> >
> > > Hello Daniel,
> > >
> > > Thanks again for the review you did, could we add a small description
> > > when UNSURE or WORK_NEEDED is set in the review table ?
> > >
> > > Or will it be best to use github comments in pull request ?
> > >
> > > I'm curious about the reason, and would like to help solve them.
> > >
> > > I will try to advance this week in the review process.
> > >
> > > Regards,
> > >
> > > Gil
> > >
> > > On 28/01/23 08:46, Daniel Watford wrote:
> > > > Turns out I was able to import the list of files into Excel and copy
> and
> > > > paste the table from Excel to Confluence.
> > > >
> > > > On Sat, 28 Jan 2023 at 08:37, Daniel Watford <da...@foomoo.co.uk>
> wrote:
> > > >
> > > > > Hi Gil,
> > > > >
> > > > > I don't think a checklist is quite enough, assuming we want to
> track
> > > the
> > > > > status of each file reviewed.
> > > > >
> > > > > From the review approach section:
> > > > >
> > > > >
> > > > >    - If in the reviewers opinion a file change will not change
> OFBiz
> > > > >    behaviour in any way they should mark the corresponding entry in
> > > the table
> > > > >    below as PASSED.
> > > > >    - If the reviewer identifies an issue with a changed file, then
> they
> > > > >    should add a comment in the PR on GitHub AND mark the
> corresponding
> > > entry
> > > > >    in the table below as WORK NEEDED.
> > > > >    - If the reviewer is unsure how to classify a changed file they
> > > should
> > > > >    mark the corresponding entry in the table below as UNSURE.
> > > > >    - In each of the above cases, the reviewer should add their name
> > > > >    against the entry in the table below.
> > > > >
> > > > > The checklist doesn't give us the opportunity to see what files
> need
> > > some
> > > > > additional help.
> > > > >
> > > > > I'm sure there must be some way of getting Confluence to produce a
> > > table
> > > > > from a list - I just don't seem to have found it yet! I'll play
> around
> > > with
> > > > > Confluence a bit more.
> > > > >
> > > > > But as mentioned before, perhaps I am making too much out of
> tracking
> > > this
> > > > > review.
> > > > >
> > > > > Thanks,
> > > > >
> > > > > Dan.
> > > > >
> > > > >
> > > > > On Fri, 27 Jan 2023 at 17:05, gil.portenseigne <
> > > > > gil.portenseigne@nereide.fr> wrote:
> > > > >
> > > > >> I got to leave, but i generated in confluence a list of check, is
> that
> > > > >> good enough ?
> > > > >>
> > > > >> Gil
> > > > >> On 27/01/23 05:41, gil.portenseigne wrote:
> > > > >> > Hello, indeed, that will generate much spam, i did some before
> > > reading
> > > > >> > your answer.
> > > > >> >
> > > > >> > I'll have a look for conluence.
> > > > >> >
> > > > >> > Gil
> > > > >> >
> > > > >> >
> > > > >> > On 27/01/23 04:14, Daniel Watford wrote:
> > > > >> > > Hi Gill and Jacques,
> > > > >> > >
> > > > >> > > I don't think we should add comments to the PR to track the
> files
> > > > >> that we
> > > > >> > > have reviewed as I think each comment will appear separately
> in
> > > the
> > > > >> PR's
> > > > >> > > conversation view.
> > > > >> > >
> > > > >> > > However, with such a large PR where we hope to get several
> > > reviewers
> > > > >> > > involved I think we do need a mechanism to track reviewed
> files.
> > > > >> > >
> > > > >> > > I created a page here - Codenarc integration review tracker -
> > > OFBiz
> > > > >> Project
> > > > >> > > Open Wiki - Apache Software Foundation
> > > > >> > > <
> > > > >>
> > >
> https://cwiki.apache.org/confluence/display/OFBIZ/Codenarc+integration+review+tracker
> > > > >> >
> > > > >> > > -
> > > > >> > > suggesting an approach.
> > > > >> > >
> > > > >> > > If the approach is acceptable then all reviewers should be
> able to
> > > > >> update
> > > > >> > > the page as we go.
> > > > >> > >
> > > > >> > > I'm stuck with finding a nice way to generate a table listing
> all
> > > the
> > > > >> > > changed files and the review status of each file. I have
> included
> > > the
> > > > >> > > commands to produce the list of files and shown some examples
> of
> > > how
> > > > >> to add
> > > > >> > > a header, but my attempts to turn that into something useful
> on a
> > > > >> > > confluence page have not been fruitful.
> > > > >> > >
> > > > >> > > So two questions.
> > > > >> > > - Is it worth coming up with a page/table to track this PR or
> am I
> > > > >> just
> > > > >> > > creating unnecessary admin work when we could use comments in
> the
> > > PR?
> > > > >> > > - Can anyone create a table in Confluence that we could use to
> > > track
> > > > >> the
> > > > >> > > review effort?
> > > > >> > >
> > > > >> > > Thanks,
> > > > >> > >
> > > > >> > > Dan.
> > > > >> > >
> > > > >> > >
> > > > >> > > On Fri, 27 Jan 2023 at 15:27, gil.portenseigne <
> > > > >> gil.portenseigne@nereide.fr>
> > > > >> > > wrote:
> > > > >> > >
> > > > >> > > > Oops, i did a fixup commit with push force that remove all
> > > comments
> > > > >> in
> > > > >> > > > the pull request... Will not do that again.
> > > > >> > > >
> > > > >> > > > I fixed the detected typo.
> > > > >> > > >
> > > > >> > > > gil
> > > > >> > > > On 27/01/23 02:56, Jacques Le Roux wrote:
> > > > >> > > > > Ah OK, sounds better indeed
> > > > >> > > > >
> > > > >> > > > > Le 27/01/2023 à 14:06, gil.portenseigne a écrit :
> > > > >> > > > > > The idea is not to modify the files, but to add a
> comment
> > > into
> > > > >> the pull
> > > > >> > > > > > request. Those allowing each reviewer to check the
> viewed
> > > > >> checkbox if a
> > > > >> > > > > > comment is present, to collapse already reviewed files.
> > > > >> > > > > >
> > > > >> > > > > > So no need further action, apart the real code
> modification
> > > > >> request,
> > > > >> > > > > > when commiting the code.
> > > > >> > > > > >
> > > > >> > > > > > On 27/01/23 12:00, Jacques Le Roux wrote:
> > > > >> > > > > > > Hi Gil, Daniel,
> > > > >> > > > > > >
> > > > >> > > > > > > I agree Gil, I just tried before seeing your message
> and
> > > came
> > > > >> to the
> > > > >> > > > same conclusion.
> > > > >> > > > > > >
> > > > >> > > > > > > With a comment at top we would need to remove it
> later,
> > > > >> right? Could
> > > > >> > > > be easy if it's the same unique words in every file.
> > > > >> > > > > > >
> > > > >> > > > > > > Jacques
> > > > >> > > > > > >
> > > > >> > > > > > > Le 27/01/2023 à 10:41, gil.portenseigne a écrit :
> > > > >> > > > > > > > Hi Daniel, Jacques,
> > > > >> > > > > > > >
> > > > >> > > > > > > > I wonders the same, the "Review changes" do not
> seems to
> > > > >> concern
> > > > >> > > > one
> > > > >> > > > > > > > file but the whole pull request, there is a review
> > > > >> checkbox, but it
> > > > >> > > > > > > > seems to be personal, i checked the first one
> > > > >> > > > > > > > (AcctgAdminServices.groovy) for testing purpose.
> > > > >> > > > > > > >
> > > > >> > > > > > > > What we could do is to add a comment at the start of
> > > each
> > > > >> file, to
> > > > >> > > > let
> > > > >> > > > > > > > others know that review job has been done.
> > > > >> > > > > > > >
> > > > >> > > > > > > > WDYT ?
> > > > >> > > > > > > >
> > > > >> > > > > > > > Gil
> > > > >> > > > > > > >
> > > > >> > > > > > > >
> > > > >> > > > > > > > On 26/01/23 07:48, Jacques Le Roux wrote:
> > > > >> > > > > > > > > Hi Daniel,
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > In "Files changed" tab*, when you select a file,
> the
> > > > >> "Review
> > > > >> > > > changes" button allows you to comment, approve or request
> > > changes
> > > > >> on this
> > > > >> > > > file.
> > > > >> > > > > > > > > I guess "approve" is what you are looking for?
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > *
> > > > >> https://github.com/apache/ofbiz-framework/pull/517/files
> > > > >> > > > > > > > >
> > > > >> > > > > > > > > Le 26/01/2023 à 17:26, Daniel Watford a écrit :
> > > > >> > > > > > > > > > Does anyone know of a way in a GitHub PR that a
> > > > >> reviewer can
> > > > >> > > > mark an
> > > > >> > > > > > > > > > individual file as reviewed-and-passed so that
> other
> > > > >> reviewers
> > > > >> > > > can skip
> > > > >> > > > > > > > > > that file?
> > > > >> > > >
> > > > >> > >
> > > > >> > >
> > > > >> > > --
> > > > >> > > Daniel Watford
> > > > >>
> > > > >>
> > > > >>
> > > > >
> > > > > --
> > > > > Daniel Watford
> > > > >
> > > >
> > > >
> > > > --
> > > > Daniel Watford
> > >
> >
> >
> > --
> > Daniel Watford
>


-- 
Daniel Watford

Re: Codenarc integration process

Posted by "gil.portenseigne" <gi...@nereide.fr>.
Indeed, i agree it is better in PR (sorry again for missing it), but i
did not find comments about the StatsSinceStart.groovy and other files
you noted as unpassed.

Did I miss something ?

Gil
On 06/02/23 09:10, Daniel Watford wrote:
> Hi Gil,
> 
> The Review Approach section of the tracking document says that comments
> should be added to the PR when marking a file as WORK_NEEDED. In my case I
> added review comments to the PR.
> 
> However I didn't specify how a reviewer should communicate why they were
> UNSURE about any particular file. I had added a comment to the PR, but
> depending on what the issue is, adding a note to the table might be
> appropriate.
> 
> I think comments in PRs are a reasonable approach since they will keep the
> comment alongside the code that needs to be worked on or discussed. I would
> rather not duplicate comments from the PR in the tracking document.
> 
> Thanks,
> 
> Dan.
> 
> On Mon, 6 Feb 2023 at 08:18, gil.portenseigne <gi...@nereide.fr>
> wrote:
> 
> > Hello Daniel,
> >
> > Thanks again for the review you did, could we add a small description
> > when UNSURE or WORK_NEEDED is set in the review table ?
> >
> > Or will it be best to use github comments in pull request ?
> >
> > I'm curious about the reason, and would like to help solve them.
> >
> > I will try to advance this week in the review process.
> >
> > Regards,
> >
> > Gil
> >
> > On 28/01/23 08:46, Daniel Watford wrote:
> > > Turns out I was able to import the list of files into Excel and copy and
> > > paste the table from Excel to Confluence.
> > >
> > > On Sat, 28 Jan 2023 at 08:37, Daniel Watford <da...@foomoo.co.uk> wrote:
> > >
> > > > Hi Gil,
> > > >
> > > > I don't think a checklist is quite enough, assuming we want to track
> > the
> > > > status of each file reviewed.
> > > >
> > > > From the review approach section:
> > > >
> > > >
> > > >    - If in the reviewers opinion a file change will not change OFBiz
> > > >    behaviour in any way they should mark the corresponding entry in
> > the table
> > > >    below as PASSED.
> > > >    - If the reviewer identifies an issue with a changed file, then they
> > > >    should add a comment in the PR on GitHub AND mark the corresponding
> > entry
> > > >    in the table below as WORK NEEDED.
> > > >    - If the reviewer is unsure how to classify a changed file they
> > should
> > > >    mark the corresponding entry in the table below as UNSURE.
> > > >    - In each of the above cases, the reviewer should add their name
> > > >    against the entry in the table below.
> > > >
> > > > The checklist doesn't give us the opportunity to see what files need
> > some
> > > > additional help.
> > > >
> > > > I'm sure there must be some way of getting Confluence to produce a
> > table
> > > > from a list - I just don't seem to have found it yet! I'll play around
> > with
> > > > Confluence a bit more.
> > > >
> > > > But as mentioned before, perhaps I am making too much out of tracking
> > this
> > > > review.
> > > >
> > > > Thanks,
> > > >
> > > > Dan.
> > > >
> > > >
> > > > On Fri, 27 Jan 2023 at 17:05, gil.portenseigne <
> > > > gil.portenseigne@nereide.fr> wrote:
> > > >
> > > >> I got to leave, but i generated in confluence a list of check, is that
> > > >> good enough ?
> > > >>
> > > >> Gil
> > > >> On 27/01/23 05:41, gil.portenseigne wrote:
> > > >> > Hello, indeed, that will generate much spam, i did some before
> > reading
> > > >> > your answer.
> > > >> >
> > > >> > I'll have a look for conluence.
> > > >> >
> > > >> > Gil
> > > >> >
> > > >> >
> > > >> > On 27/01/23 04:14, Daniel Watford wrote:
> > > >> > > Hi Gill and Jacques,
> > > >> > >
> > > >> > > I don't think we should add comments to the PR to track the files
> > > >> that we
> > > >> > > have reviewed as I think each comment will appear separately in
> > the
> > > >> PR's
> > > >> > > conversation view.
> > > >> > >
> > > >> > > However, with such a large PR where we hope to get several
> > reviewers
> > > >> > > involved I think we do need a mechanism to track reviewed files.
> > > >> > >
> > > >> > > I created a page here - Codenarc integration review tracker -
> > OFBiz
> > > >> Project
> > > >> > > Open Wiki - Apache Software Foundation
> > > >> > > <
> > > >>
> > https://cwiki.apache.org/confluence/display/OFBIZ/Codenarc+integration+review+tracker
> > > >> >
> > > >> > > -
> > > >> > > suggesting an approach.
> > > >> > >
> > > >> > > If the approach is acceptable then all reviewers should be able to
> > > >> update
> > > >> > > the page as we go.
> > > >> > >
> > > >> > > I'm stuck with finding a nice way to generate a table listing all
> > the
> > > >> > > changed files and the review status of each file. I have included
> > the
> > > >> > > commands to produce the list of files and shown some examples of
> > how
> > > >> to add
> > > >> > > a header, but my attempts to turn that into something useful on a
> > > >> > > confluence page have not been fruitful.
> > > >> > >
> > > >> > > So two questions.
> > > >> > > - Is it worth coming up with a page/table to track this PR or am I
> > > >> just
> > > >> > > creating unnecessary admin work when we could use comments in the
> > PR?
> > > >> > > - Can anyone create a table in Confluence that we could use to
> > track
> > > >> the
> > > >> > > review effort?
> > > >> > >
> > > >> > > Thanks,
> > > >> > >
> > > >> > > Dan.
> > > >> > >
> > > >> > >
> > > >> > > On Fri, 27 Jan 2023 at 15:27, gil.portenseigne <
> > > >> gil.portenseigne@nereide.fr>
> > > >> > > wrote:
> > > >> > >
> > > >> > > > Oops, i did a fixup commit with push force that remove all
> > comments
> > > >> in
> > > >> > > > the pull request... Will not do that again.
> > > >> > > >
> > > >> > > > I fixed the detected typo.
> > > >> > > >
> > > >> > > > gil
> > > >> > > > On 27/01/23 02:56, Jacques Le Roux wrote:
> > > >> > > > > Ah OK, sounds better indeed
> > > >> > > > >
> > > >> > > > > Le 27/01/2023 à 14:06, gil.portenseigne a écrit :
> > > >> > > > > > The idea is not to modify the files, but to add a comment
> > into
> > > >> the pull
> > > >> > > > > > request. Those allowing each reviewer to check the viewed
> > > >> checkbox if a
> > > >> > > > > > comment is present, to collapse already reviewed files.
> > > >> > > > > >
> > > >> > > > > > So no need further action, apart the real code modification
> > > >> request,
> > > >> > > > > > when commiting the code.
> > > >> > > > > >
> > > >> > > > > > On 27/01/23 12:00, Jacques Le Roux wrote:
> > > >> > > > > > > Hi Gil, Daniel,
> > > >> > > > > > >
> > > >> > > > > > > I agree Gil, I just tried before seeing your message and
> > came
> > > >> to the
> > > >> > > > same conclusion.
> > > >> > > > > > >
> > > >> > > > > > > With a comment at top we would need to remove it later,
> > > >> right? Could
> > > >> > > > be easy if it's the same unique words in every file.
> > > >> > > > > > >
> > > >> > > > > > > Jacques
> > > >> > > > > > >
> > > >> > > > > > > Le 27/01/2023 à 10:41, gil.portenseigne a écrit :
> > > >> > > > > > > > Hi Daniel, Jacques,
> > > >> > > > > > > >
> > > >> > > > > > > > I wonders the same, the "Review changes" do not seems to
> > > >> concern
> > > >> > > > one
> > > >> > > > > > > > file but the whole pull request, there is a review
> > > >> checkbox, but it
> > > >> > > > > > > > seems to be personal, i checked the first one
> > > >> > > > > > > > (AcctgAdminServices.groovy) for testing purpose.
> > > >> > > > > > > >
> > > >> > > > > > > > What we could do is to add a comment at the start of
> > each
> > > >> file, to
> > > >> > > > let
> > > >> > > > > > > > others know that review job has been done.
> > > >> > > > > > > >
> > > >> > > > > > > > WDYT ?
> > > >> > > > > > > >
> > > >> > > > > > > > Gil
> > > >> > > > > > > >
> > > >> > > > > > > >
> > > >> > > > > > > > On 26/01/23 07:48, Jacques Le Roux wrote:
> > > >> > > > > > > > > Hi Daniel,
> > > >> > > > > > > > >
> > > >> > > > > > > > > In "Files changed" tab*, when you select a file, the
> > > >> "Review
> > > >> > > > changes" button allows you to comment, approve or request
> > changes
> > > >> on this
> > > >> > > > file.
> > > >> > > > > > > > > I guess "approve" is what you are looking for?
> > > >> > > > > > > > >
> > > >> > > > > > > > > *
> > > >> https://github.com/apache/ofbiz-framework/pull/517/files
> > > >> > > > > > > > >
> > > >> > > > > > > > > Le 26/01/2023 à 17:26, Daniel Watford a écrit :
> > > >> > > > > > > > > > Does anyone know of a way in a GitHub PR that a
> > > >> reviewer can
> > > >> > > > mark an
> > > >> > > > > > > > > > individual file as reviewed-and-passed so that other
> > > >> reviewers
> > > >> > > > can skip
> > > >> > > > > > > > > > that file?
> > > >> > > >
> > > >> > >
> > > >> > >
> > > >> > > --
> > > >> > > Daniel Watford
> > > >>
> > > >>
> > > >>
> > > >
> > > > --
> > > > Daniel Watford
> > > >
> > >
> > >
> > > --
> > > Daniel Watford
> >
> 
> 
> -- 
> Daniel Watford

Re: Codenarc integration process

Posted by Jacques Le Roux <ja...@les7arts.com>.
Le 06/02/2023 à 09:18, gil.portenseigne a écrit :
> I will try to advance this week in the review process.

Hi Gil, Daniel, All,

I'll try to the apply the 10 min principle to the review. Reviewing 5 documents/day should allow to do my part (150) in roughly a month :)

Jacques


Re: Codenarc integration process

Posted by Daniel Watford <da...@foomoo.co.uk>.
Hi Gil,

The Review Approach section of the tracking document says that comments
should be added to the PR when marking a file as WORK_NEEDED. In my case I
added review comments to the PR.

However I didn't specify how a reviewer should communicate why they were
UNSURE about any particular file. I had added a comment to the PR, but
depending on what the issue is, adding a note to the table might be
appropriate.

I think comments in PRs are a reasonable approach since they will keep the
comment alongside the code that needs to be worked on or discussed. I would
rather not duplicate comments from the PR in the tracking document.

Thanks,

Dan.

On Mon, 6 Feb 2023 at 08:18, gil.portenseigne <gi...@nereide.fr>
wrote:

> Hello Daniel,
>
> Thanks again for the review you did, could we add a small description
> when UNSURE or WORK_NEEDED is set in the review table ?
>
> Or will it be best to use github comments in pull request ?
>
> I'm curious about the reason, and would like to help solve them.
>
> I will try to advance this week in the review process.
>
> Regards,
>
> Gil
>
> On 28/01/23 08:46, Daniel Watford wrote:
> > Turns out I was able to import the list of files into Excel and copy and
> > paste the table from Excel to Confluence.
> >
> > On Sat, 28 Jan 2023 at 08:37, Daniel Watford <da...@foomoo.co.uk> wrote:
> >
> > > Hi Gil,
> > >
> > > I don't think a checklist is quite enough, assuming we want to track
> the
> > > status of each file reviewed.
> > >
> > > From the review approach section:
> > >
> > >
> > >    - If in the reviewers opinion a file change will not change OFBiz
> > >    behaviour in any way they should mark the corresponding entry in
> the table
> > >    below as PASSED.
> > >    - If the reviewer identifies an issue with a changed file, then they
> > >    should add a comment in the PR on GitHub AND mark the corresponding
> entry
> > >    in the table below as WORK NEEDED.
> > >    - If the reviewer is unsure how to classify a changed file they
> should
> > >    mark the corresponding entry in the table below as UNSURE.
> > >    - In each of the above cases, the reviewer should add their name
> > >    against the entry in the table below.
> > >
> > > The checklist doesn't give us the opportunity to see what files need
> some
> > > additional help.
> > >
> > > I'm sure there must be some way of getting Confluence to produce a
> table
> > > from a list - I just don't seem to have found it yet! I'll play around
> with
> > > Confluence a bit more.
> > >
> > > But as mentioned before, perhaps I am making too much out of tracking
> this
> > > review.
> > >
> > > Thanks,
> > >
> > > Dan.
> > >
> > >
> > > On Fri, 27 Jan 2023 at 17:05, gil.portenseigne <
> > > gil.portenseigne@nereide.fr> wrote:
> > >
> > >> I got to leave, but i generated in confluence a list of check, is that
> > >> good enough ?
> > >>
> > >> Gil
> > >> On 27/01/23 05:41, gil.portenseigne wrote:
> > >> > Hello, indeed, that will generate much spam, i did some before
> reading
> > >> > your answer.
> > >> >
> > >> > I'll have a look for conluence.
> > >> >
> > >> > Gil
> > >> >
> > >> >
> > >> > On 27/01/23 04:14, Daniel Watford wrote:
> > >> > > Hi Gill and Jacques,
> > >> > >
> > >> > > I don't think we should add comments to the PR to track the files
> > >> that we
> > >> > > have reviewed as I think each comment will appear separately in
> the
> > >> PR's
> > >> > > conversation view.
> > >> > >
> > >> > > However, with such a large PR where we hope to get several
> reviewers
> > >> > > involved I think we do need a mechanism to track reviewed files.
> > >> > >
> > >> > > I created a page here - Codenarc integration review tracker -
> OFBiz
> > >> Project
> > >> > > Open Wiki - Apache Software Foundation
> > >> > > <
> > >>
> https://cwiki.apache.org/confluence/display/OFBIZ/Codenarc+integration+review+tracker
> > >> >
> > >> > > -
> > >> > > suggesting an approach.
> > >> > >
> > >> > > If the approach is acceptable then all reviewers should be able to
> > >> update
> > >> > > the page as we go.
> > >> > >
> > >> > > I'm stuck with finding a nice way to generate a table listing all
> the
> > >> > > changed files and the review status of each file. I have included
> the
> > >> > > commands to produce the list of files and shown some examples of
> how
> > >> to add
> > >> > > a header, but my attempts to turn that into something useful on a
> > >> > > confluence page have not been fruitful.
> > >> > >
> > >> > > So two questions.
> > >> > > - Is it worth coming up with a page/table to track this PR or am I
> > >> just
> > >> > > creating unnecessary admin work when we could use comments in the
> PR?
> > >> > > - Can anyone create a table in Confluence that we could use to
> track
> > >> the
> > >> > > review effort?
> > >> > >
> > >> > > Thanks,
> > >> > >
> > >> > > Dan.
> > >> > >
> > >> > >
> > >> > > On Fri, 27 Jan 2023 at 15:27, gil.portenseigne <
> > >> gil.portenseigne@nereide.fr>
> > >> > > wrote:
> > >> > >
> > >> > > > Oops, i did a fixup commit with push force that remove all
> comments
> > >> in
> > >> > > > the pull request... Will not do that again.
> > >> > > >
> > >> > > > I fixed the detected typo.
> > >> > > >
> > >> > > > gil
> > >> > > > On 27/01/23 02:56, Jacques Le Roux wrote:
> > >> > > > > Ah OK, sounds better indeed
> > >> > > > >
> > >> > > > > Le 27/01/2023 à 14:06, gil.portenseigne a écrit :
> > >> > > > > > The idea is not to modify the files, but to add a comment
> into
> > >> the pull
> > >> > > > > > request. Those allowing each reviewer to check the viewed
> > >> checkbox if a
> > >> > > > > > comment is present, to collapse already reviewed files.
> > >> > > > > >
> > >> > > > > > So no need further action, apart the real code modification
> > >> request,
> > >> > > > > > when commiting the code.
> > >> > > > > >
> > >> > > > > > On 27/01/23 12:00, Jacques Le Roux wrote:
> > >> > > > > > > Hi Gil, Daniel,
> > >> > > > > > >
> > >> > > > > > > I agree Gil, I just tried before seeing your message and
> came
> > >> to the
> > >> > > > same conclusion.
> > >> > > > > > >
> > >> > > > > > > With a comment at top we would need to remove it later,
> > >> right? Could
> > >> > > > be easy if it's the same unique words in every file.
> > >> > > > > > >
> > >> > > > > > > Jacques
> > >> > > > > > >
> > >> > > > > > > Le 27/01/2023 à 10:41, gil.portenseigne a écrit :
> > >> > > > > > > > Hi Daniel, Jacques,
> > >> > > > > > > >
> > >> > > > > > > > I wonders the same, the "Review changes" do not seems to
> > >> concern
> > >> > > > one
> > >> > > > > > > > file but the whole pull request, there is a review
> > >> checkbox, but it
> > >> > > > > > > > seems to be personal, i checked the first one
> > >> > > > > > > > (AcctgAdminServices.groovy) for testing purpose.
> > >> > > > > > > >
> > >> > > > > > > > What we could do is to add a comment at the start of
> each
> > >> file, to
> > >> > > > let
> > >> > > > > > > > others know that review job has been done.
> > >> > > > > > > >
> > >> > > > > > > > WDYT ?
> > >> > > > > > > >
> > >> > > > > > > > Gil
> > >> > > > > > > >
> > >> > > > > > > >
> > >> > > > > > > > On 26/01/23 07:48, Jacques Le Roux wrote:
> > >> > > > > > > > > Hi Daniel,
> > >> > > > > > > > >
> > >> > > > > > > > > In "Files changed" tab*, when you select a file, the
> > >> "Review
> > >> > > > changes" button allows you to comment, approve or request
> changes
> > >> on this
> > >> > > > file.
> > >> > > > > > > > > I guess "approve" is what you are looking for?
> > >> > > > > > > > >
> > >> > > > > > > > > *
> > >> https://github.com/apache/ofbiz-framework/pull/517/files
> > >> > > > > > > > >
> > >> > > > > > > > > Le 26/01/2023 à 17:26, Daniel Watford a écrit :
> > >> > > > > > > > > > Does anyone know of a way in a GitHub PR that a
> > >> reviewer can
> > >> > > > mark an
> > >> > > > > > > > > > individual file as reviewed-and-passed so that other
> > >> reviewers
> > >> > > > can skip
> > >> > > > > > > > > > that file?
> > >> > > >
> > >> > >
> > >> > >
> > >> > > --
> > >> > > Daniel Watford
> > >>
> > >>
> > >>
> > >
> > > --
> > > Daniel Watford
> > >
> >
> >
> > --
> > Daniel Watford
>


-- 
Daniel Watford