How to deal with module review without modifications

[Dave Schoorl <ds...@bkwi.nl>]

Hi all,

When you check a module for license headers and suspicious files (files that maybe were not owned by Oracle), make changes and do a Pull Request (PR), the PR is checked by one or more committers. On the other hand, when the person checking a module thinks no modifications are needed, no PR is made and no code review is done (obviously). However, review of the module check does not seem to take place either. What do we think of that?

E.g., Geertjan is wotking his ass off, checking one module after another, but making very little PR's compared to the number of modules he processes. When I review a couple of these modules, I think there might be some oversights. E.g.

api.intent has a Bundle.properties with an Oracle license header

api.progress has a apichanges.xml with an Oracle license header

api.progress.nb has aapichanges.xml with an Oracle license header

I would expect those Oracle license headers to be removed or replaced with Apache license headers.


How important is it to get the license headers right? Do we need a peer review on modules that seem okay? A second name in the name column on the wiki page, confirming that no changes are needed?

Please advise.


/Dave


@Geertjan: sorry to have used you as an example.

Re: How to deal with module review without modifications

[Geertjan Wielenga <ge...@googlemail.com>]

Right. And in that case the choice is between extending the tool to capture
that variation or to fix it manually, possibly hundreds of times. Don't
know the 'right' answer or if there is one, but those are the two solutions.

Gj

On Wed, Sep 27, 2017 at 10:34 AM, Emilian Bold <em...@gmail.com>
wrote:

> Well, most Bundle.properties have been automatically updated but if some
> remained I guess the header was slightly off?
>
>
> --emi
>
> On Wed, Sep 27, 2017 at 11:30 AM, Geertjan Wielenga <
> geertjan.wielenga@googlemail.com> wrote:
>
> > To me, a "Bundle.properties" is also something that should be solved
> > centrally, since that's a general problem, rather than being a unique or
> > special file type in the module. That's something that should be added to
> > the central problems list, in my understanding of it.
> >
> > Gj
> >
> > On Wed, Sep 27, 2017 at 10:26 AM, Emilian Bold <em...@gmail.com>
> > wrote:
> >
> > > That's the way I see it.
> > >
> > > Note that Dave mentioned some oversights. For example,
> Bundle.properties
> > > with an Oracle license header.
> > >
> > > Such a thing should be easily double-checked with some 'grep -R Oracle
> '
> > at
> > > some point.
> > >
> > >
> > > --emi
> > >
> > > On Wed, Sep 27, 2017 at 11:11 AM, Geertjan Wielenga <
> > > geertjan.wielenga@googlemail.com> wrote:
> > >
> > > > OK, so, if everything is done except for the central things, then
> it's
> > > > done. I'll revert my "To dos" back to "Dones" and add a note re this
> to
> > > the
> > > > status section at the top of the page.
> > > >
> > > > Gj
> > > >
> > > > On Wed, Sep 27, 2017 at 10:08 AM, Emilian Bold <
> emilian.bold@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > I believe modules that still have .form files or other "Problems to
> > be
> > > > > solved centrally" should be marked as "Done".
> > > > >
> > > > > The whole point is to look at everything else. Those problems will
> be
> > > > > solved... centrally.
> > > > >
> > > > >
> > > > > --emi
> > > > >
> > > > > On Wed, Sep 27, 2017 at 11:02 AM, Geertjan Wielenga <
> > > > > geertjan.wielenga@googlemail.com> wrote:
> > > > >
> > > > > > OK, I have marked all mine as "To do", since I am relying on the
> > > > central
> > > > > > problems being handled via the tool, i.e., I think all "form"
> > files,
> > > > for
> > > > > > example, should be processed via the tool, rather than doing them
> > > > > > individually and manually per module.
> > > > > >
> > > > > > I think a peer review would be good, and yes, it is very
> important
> > to
> > > > get
> > > > > > the license headers right. A peer review column would be great, I
> > > > think,
> > > > > > i.e., when a row is marked as Done someone could check that and
> > > provide
> > > > > any
> > > > > > comments in the peer review column.
> > > > > >
> > > > > > Gj
> > > > > >
> > > > > > On Wed, Sep 27, 2017 at 12:32 AM, Geertjan Wielenga <
> > > > > > geertjan.wielenga@googlemail.com> wrote:
> > > > > >
> > > > > > > Actually, that means I shouldn't mark those modules as "Done"
> but
> > > as
> > > > > "To
> > > > > > > do".
> > > > > > >
> > > > > > > Gj
> > > > > > >
> > > > > > > On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
> > > > > > > geertjan.wielenga@googlemail.com> wrote:
> > > > > > >
> > > > > > >> My preference is for trying to handle those common things via
> > the
> > > > > tool,
> > > > > > >> i.e., rather than manually fixing apichanges.xml by hand, the
> > > point
> > > > to
> > > > > > me
> > > > > > >> is to identify that that needs to be fixed, or XML files in
> > > general,
> > > > > and
> > > > > > >> add them to the generally problematic list at the top of the
> > page.
> > > > > > >>
> > > > > > >> Gj
> > > > > > >>
> > > > > > >> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <
> > dschoorl@bkwi.nl>
> > > > > > wrote:
> > > > > > >>
> > > > > > >>> Hi all,
> > > > > > >>>
> > > > > > >>> When you check a module for license headers and suspicious
> > files
> > > > > (files
> > > > > > >>> that maybe were not owned by Oracle), make changes and do a
> > Pull
> > > > > > Request
> > > > > > >>> (PR), the PR is checked by one or more committers. On the
> other
> > > > hand,
> > > > > > when
> > > > > > >>> the person checking a module thinks no modifications are
> > needed,
> > > no
> > > > > PR
> > > > > > is
> > > > > > >>> made and no code review is done (obviously). However, review
> of
> > > the
> > > > > > module
> > > > > > >>> check does not seem to take place either. What do we think of
> > > that?
> > > > > > >>>
> > > > > > >>> E.g., Geertjan is wotking his ass off, checking one module
> > after
> > > > > > >>> another, but making very little PR's compared to the number
> of
> > > > > modules
> > > > > > he
> > > > > > >>> processes. When I review a couple of these modules, I think
> > there
> > > > > > might be
> > > > > > >>> some oversights. E.g.
> > > > > > >>>
> > > > > > >>> api.intent has a Bundle.properties with an Oracle license
> > header
> > > > > > >>>
> > > > > > >>> api.progress has a apichanges.xml with an Oracle license
> header
> > > > > > >>>
> > > > > > >>> api.progress.nb has aapichanges.xml with an Oracle license
> > header
> > > > > > >>>
> > > > > > >>> I would expect those Oracle license headers to be removed or
> > > > replaced
> > > > > > >>> with Apache license headers.
> > > > > > >>>
> > > > > > >>>
> > > > > > >>> How important is it to get the license headers right? Do we
> > need
> > > a
> > > > > peer
> > > > > > >>> review on modules that seem okay? A second name in the name
> > > column
> > > > on
> > > > > > the
> > > > > > >>> wiki page, confirming that no changes are needed?
> > > > > > >>>
> > > > > > >>> Please advise.
> > > > > > >>>
> > > > > > >>>
> > > > > > >>> /Dave
> > > > > > >>>
> > > > > > >>>
> > > > > > >>> @Geertjan: sorry to have used you as an example.
> > > > > > >>>
> > > > > > >>
> > > > > > >>
> > > > > > >
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: How to deal with module review without modifications

[Emilian Bold <em...@gmail.com>]

Well, most Bundle.properties have been automatically updated but if some
remained I guess the header was slightly off?


--emi

On Wed, Sep 27, 2017 at 11:30 AM, Geertjan Wielenga <
geertjan.wielenga@googlemail.com> wrote:

> To me, a "Bundle.properties" is also something that should be solved
> centrally, since that's a general problem, rather than being a unique or
> special file type in the module. That's something that should be added to
> the central problems list, in my understanding of it.
>
> Gj
>
> On Wed, Sep 27, 2017 at 10:26 AM, Emilian Bold <em...@gmail.com>
> wrote:
>
> > That's the way I see it.
> >
> > Note that Dave mentioned some oversights. For example, Bundle.properties
> > with an Oracle license header.
> >
> > Such a thing should be easily double-checked with some 'grep -R Oracle '
> at
> > some point.
> >
> >
> > --emi
> >
> > On Wed, Sep 27, 2017 at 11:11 AM, Geertjan Wielenga <
> > geertjan.wielenga@googlemail.com> wrote:
> >
> > > OK, so, if everything is done except for the central things, then it's
> > > done. I'll revert my "To dos" back to "Dones" and add a note re this to
> > the
> > > status section at the top of the page.
> > >
> > > Gj
> > >
> > > On Wed, Sep 27, 2017 at 10:08 AM, Emilian Bold <emilian.bold@gmail.com
> >
> > > wrote:
> > >
> > > > I believe modules that still have .form files or other "Problems to
> be
> > > > solved centrally" should be marked as "Done".
> > > >
> > > > The whole point is to look at everything else. Those problems will be
> > > > solved... centrally.
> > > >
> > > >
> > > > --emi
> > > >
> > > > On Wed, Sep 27, 2017 at 11:02 AM, Geertjan Wielenga <
> > > > geertjan.wielenga@googlemail.com> wrote:
> > > >
> > > > > OK, I have marked all mine as "To do", since I am relying on the
> > > central
> > > > > problems being handled via the tool, i.e., I think all "form"
> files,
> > > for
> > > > > example, should be processed via the tool, rather than doing them
> > > > > individually and manually per module.
> > > > >
> > > > > I think a peer review would be good, and yes, it is very important
> to
> > > get
> > > > > the license headers right. A peer review column would be great, I
> > > think,
> > > > > i.e., when a row is marked as Done someone could check that and
> > provide
> > > > any
> > > > > comments in the peer review column.
> > > > >
> > > > > Gj
> > > > >
> > > > > On Wed, Sep 27, 2017 at 12:32 AM, Geertjan Wielenga <
> > > > > geertjan.wielenga@googlemail.com> wrote:
> > > > >
> > > > > > Actually, that means I shouldn't mark those modules as "Done" but
> > as
> > > > "To
> > > > > > do".
> > > > > >
> > > > > > Gj
> > > > > >
> > > > > > On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
> > > > > > geertjan.wielenga@googlemail.com> wrote:
> > > > > >
> > > > > >> My preference is for trying to handle those common things via
> the
> > > > tool,
> > > > > >> i.e., rather than manually fixing apichanges.xml by hand, the
> > point
> > > to
> > > > > me
> > > > > >> is to identify that that needs to be fixed, or XML files in
> > general,
> > > > and
> > > > > >> add them to the generally problematic list at the top of the
> page.
> > > > > >>
> > > > > >> Gj
> > > > > >>
> > > > > >> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <
> dschoorl@bkwi.nl>
> > > > > wrote:
> > > > > >>
> > > > > >>> Hi all,
> > > > > >>>
> > > > > >>> When you check a module for license headers and suspicious
> files
> > > > (files
> > > > > >>> that maybe were not owned by Oracle), make changes and do a
> Pull
> > > > > Request
> > > > > >>> (PR), the PR is checked by one or more committers. On the other
> > > hand,
> > > > > when
> > > > > >>> the person checking a module thinks no modifications are
> needed,
> > no
> > > > PR
> > > > > is
> > > > > >>> made and no code review is done (obviously). However, review of
> > the
> > > > > module
> > > > > >>> check does not seem to take place either. What do we think of
> > that?
> > > > > >>>
> > > > > >>> E.g., Geertjan is wotking his ass off, checking one module
> after
> > > > > >>> another, but making very little PR's compared to the number of
> > > > modules
> > > > > he
> > > > > >>> processes. When I review a couple of these modules, I think
> there
> > > > > might be
> > > > > >>> some oversights. E.g.
> > > > > >>>
> > > > > >>> api.intent has a Bundle.properties with an Oracle license
> header
> > > > > >>>
> > > > > >>> api.progress has a apichanges.xml with an Oracle license header
> > > > > >>>
> > > > > >>> api.progress.nb has aapichanges.xml with an Oracle license
> header
> > > > > >>>
> > > > > >>> I would expect those Oracle license headers to be removed or
> > > replaced
> > > > > >>> with Apache license headers.
> > > > > >>>
> > > > > >>>
> > > > > >>> How important is it to get the license headers right? Do we
> need
> > a
> > > > peer
> > > > > >>> review on modules that seem okay? A second name in the name
> > column
> > > on
> > > > > the
> > > > > >>> wiki page, confirming that no changes are needed?
> > > > > >>>
> > > > > >>> Please advise.
> > > > > >>>
> > > > > >>>
> > > > > >>> /Dave
> > > > > >>>
> > > > > >>>
> > > > > >>> @Geertjan: sorry to have used you as an example.
> > > > > >>>
> > > > > >>
> > > > > >>
> > > > > >
> > > > >
> > > >
> > >
> >
>

Re: How to deal with module review without modifications

[Geertjan Wielenga <ge...@googlemail.com>]

To me, a "Bundle.properties" is also something that should be solved
centrally, since that's a general problem, rather than being a unique or
special file type in the module. That's something that should be added to
the central problems list, in my understanding of it.

Gj

On Wed, Sep 27, 2017 at 10:26 AM, Emilian Bold <em...@gmail.com>
wrote:

> That's the way I see it.
>
> Note that Dave mentioned some oversights. For example, Bundle.properties
> with an Oracle license header.
>
> Such a thing should be easily double-checked with some 'grep -R Oracle ' at
> some point.
>
>
> --emi
>
> On Wed, Sep 27, 2017 at 11:11 AM, Geertjan Wielenga <
> geertjan.wielenga@googlemail.com> wrote:
>
> > OK, so, if everything is done except for the central things, then it's
> > done. I'll revert my "To dos" back to "Dones" and add a note re this to
> the
> > status section at the top of the page.
> >
> > Gj
> >
> > On Wed, Sep 27, 2017 at 10:08 AM, Emilian Bold <em...@gmail.com>
> > wrote:
> >
> > > I believe modules that still have .form files or other "Problems to be
> > > solved centrally" should be marked as "Done".
> > >
> > > The whole point is to look at everything else. Those problems will be
> > > solved... centrally.
> > >
> > >
> > > --emi
> > >
> > > On Wed, Sep 27, 2017 at 11:02 AM, Geertjan Wielenga <
> > > geertjan.wielenga@googlemail.com> wrote:
> > >
> > > > OK, I have marked all mine as "To do", since I am relying on the
> > central
> > > > problems being handled via the tool, i.e., I think all "form" files,
> > for
> > > > example, should be processed via the tool, rather than doing them
> > > > individually and manually per module.
> > > >
> > > > I think a peer review would be good, and yes, it is very important to
> > get
> > > > the license headers right. A peer review column would be great, I
> > think,
> > > > i.e., when a row is marked as Done someone could check that and
> provide
> > > any
> > > > comments in the peer review column.
> > > >
> > > > Gj
> > > >
> > > > On Wed, Sep 27, 2017 at 12:32 AM, Geertjan Wielenga <
> > > > geertjan.wielenga@googlemail.com> wrote:
> > > >
> > > > > Actually, that means I shouldn't mark those modules as "Done" but
> as
> > > "To
> > > > > do".
> > > > >
> > > > > Gj
> > > > >
> > > > > On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
> > > > > geertjan.wielenga@googlemail.com> wrote:
> > > > >
> > > > >> My preference is for trying to handle those common things via the
> > > tool,
> > > > >> i.e., rather than manually fixing apichanges.xml by hand, the
> point
> > to
> > > > me
> > > > >> is to identify that that needs to be fixed, or XML files in
> general,
> > > and
> > > > >> add them to the generally problematic list at the top of the page.
> > > > >>
> > > > >> Gj
> > > > >>
> > > > >> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <ds...@bkwi.nl>
> > > > wrote:
> > > > >>
> > > > >>> Hi all,
> > > > >>>
> > > > >>> When you check a module for license headers and suspicious files
> > > (files
> > > > >>> that maybe were not owned by Oracle), make changes and do a Pull
> > > > Request
> > > > >>> (PR), the PR is checked by one or more committers. On the other
> > hand,
> > > > when
> > > > >>> the person checking a module thinks no modifications are needed,
> no
> > > PR
> > > > is
> > > > >>> made and no code review is done (obviously). However, review of
> the
> > > > module
> > > > >>> check does not seem to take place either. What do we think of
> that?
> > > > >>>
> > > > >>> E.g., Geertjan is wotking his ass off, checking one module after
> > > > >>> another, but making very little PR's compared to the number of
> > > modules
> > > > he
> > > > >>> processes. When I review a couple of these modules, I think there
> > > > might be
> > > > >>> some oversights. E.g.
> > > > >>>
> > > > >>> api.intent has a Bundle.properties with an Oracle license header
> > > > >>>
> > > > >>> api.progress has a apichanges.xml with an Oracle license header
> > > > >>>
> > > > >>> api.progress.nb has aapichanges.xml with an Oracle license header
> > > > >>>
> > > > >>> I would expect those Oracle license headers to be removed or
> > replaced
> > > > >>> with Apache license headers.
> > > > >>>
> > > > >>>
> > > > >>> How important is it to get the license headers right? Do we need
> a
> > > peer
> > > > >>> review on modules that seem okay? A second name in the name
> column
> > on
> > > > the
> > > > >>> wiki page, confirming that no changes are needed?
> > > > >>>
> > > > >>> Please advise.
> > > > >>>
> > > > >>>
> > > > >>> /Dave
> > > > >>>
> > > > >>>
> > > > >>> @Geertjan: sorry to have used you as an example.
> > > > >>>
> > > > >>
> > > > >>
> > > > >
> > > >
> > >
> >
>

Re: How to deal with module review without modifications

[Emilian Bold <em...@gmail.com>]

That's the way I see it.

Note that Dave mentioned some oversights. For example, Bundle.properties
with an Oracle license header.

Such a thing should be easily double-checked with some 'grep -R Oracle ' at
some point.


--emi

On Wed, Sep 27, 2017 at 11:11 AM, Geertjan Wielenga <
geertjan.wielenga@googlemail.com> wrote:

> OK, so, if everything is done except for the central things, then it's
> done. I'll revert my "To dos" back to "Dones" and add a note re this to the
> status section at the top of the page.
>
> Gj
>
> On Wed, Sep 27, 2017 at 10:08 AM, Emilian Bold <em...@gmail.com>
> wrote:
>
> > I believe modules that still have .form files or other "Problems to be
> > solved centrally" should be marked as "Done".
> >
> > The whole point is to look at everything else. Those problems will be
> > solved... centrally.
> >
> >
> > --emi
> >
> > On Wed, Sep 27, 2017 at 11:02 AM, Geertjan Wielenga <
> > geertjan.wielenga@googlemail.com> wrote:
> >
> > > OK, I have marked all mine as "To do", since I am relying on the
> central
> > > problems being handled via the tool, i.e., I think all "form" files,
> for
> > > example, should be processed via the tool, rather than doing them
> > > individually and manually per module.
> > >
> > > I think a peer review would be good, and yes, it is very important to
> get
> > > the license headers right. A peer review column would be great, I
> think,
> > > i.e., when a row is marked as Done someone could check that and provide
> > any
> > > comments in the peer review column.
> > >
> > > Gj
> > >
> > > On Wed, Sep 27, 2017 at 12:32 AM, Geertjan Wielenga <
> > > geertjan.wielenga@googlemail.com> wrote:
> > >
> > > > Actually, that means I shouldn't mark those modules as "Done" but as
> > "To
> > > > do".
> > > >
> > > > Gj
> > > >
> > > > On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
> > > > geertjan.wielenga@googlemail.com> wrote:
> > > >
> > > >> My preference is for trying to handle those common things via the
> > tool,
> > > >> i.e., rather than manually fixing apichanges.xml by hand, the point
> to
> > > me
> > > >> is to identify that that needs to be fixed, or XML files in general,
> > and
> > > >> add them to the generally problematic list at the top of the page.
> > > >>
> > > >> Gj
> > > >>
> > > >> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <ds...@bkwi.nl>
> > > wrote:
> > > >>
> > > >>> Hi all,
> > > >>>
> > > >>> When you check a module for license headers and suspicious files
> > (files
> > > >>> that maybe were not owned by Oracle), make changes and do a Pull
> > > Request
> > > >>> (PR), the PR is checked by one or more committers. On the other
> hand,
> > > when
> > > >>> the person checking a module thinks no modifications are needed, no
> > PR
> > > is
> > > >>> made and no code review is done (obviously). However, review of the
> > > module
> > > >>> check does not seem to take place either. What do we think of that?
> > > >>>
> > > >>> E.g., Geertjan is wotking his ass off, checking one module after
> > > >>> another, but making very little PR's compared to the number of
> > modules
> > > he
> > > >>> processes. When I review a couple of these modules, I think there
> > > might be
> > > >>> some oversights. E.g.
> > > >>>
> > > >>> api.intent has a Bundle.properties with an Oracle license header
> > > >>>
> > > >>> api.progress has a apichanges.xml with an Oracle license header
> > > >>>
> > > >>> api.progress.nb has aapichanges.xml with an Oracle license header
> > > >>>
> > > >>> I would expect those Oracle license headers to be removed or
> replaced
> > > >>> with Apache license headers.
> > > >>>
> > > >>>
> > > >>> How important is it to get the license headers right? Do we need a
> > peer
> > > >>> review on modules that seem okay? A second name in the name column
> on
> > > the
> > > >>> wiki page, confirming that no changes are needed?
> > > >>>
> > > >>> Please advise.
> > > >>>
> > > >>>
> > > >>> /Dave
> > > >>>
> > > >>>
> > > >>> @Geertjan: sorry to have used you as an example.
> > > >>>
> > > >>
> > > >>
> > > >
> > >
> >
>

Re: How to deal with module review without modifications

[Geertjan Wielenga <ge...@googlemail.com>]

OK, so, if everything is done except for the central things, then it's
done. I'll revert my "To dos" back to "Dones" and add a note re this to the
status section at the top of the page.

Gj

On Wed, Sep 27, 2017 at 10:08 AM, Emilian Bold <em...@gmail.com>
wrote:

> I believe modules that still have .form files or other "Problems to be
> solved centrally" should be marked as "Done".
>
> The whole point is to look at everything else. Those problems will be
> solved... centrally.
>
>
> --emi
>
> On Wed, Sep 27, 2017 at 11:02 AM, Geertjan Wielenga <
> geertjan.wielenga@googlemail.com> wrote:
>
> > OK, I have marked all mine as "To do", since I am relying on the central
> > problems being handled via the tool, i.e., I think all "form" files, for
> > example, should be processed via the tool, rather than doing them
> > individually and manually per module.
> >
> > I think a peer review would be good, and yes, it is very important to get
> > the license headers right. A peer review column would be great, I think,
> > i.e., when a row is marked as Done someone could check that and provide
> any
> > comments in the peer review column.
> >
> > Gj
> >
> > On Wed, Sep 27, 2017 at 12:32 AM, Geertjan Wielenga <
> > geertjan.wielenga@googlemail.com> wrote:
> >
> > > Actually, that means I shouldn't mark those modules as "Done" but as
> "To
> > > do".
> > >
> > > Gj
> > >
> > > On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
> > > geertjan.wielenga@googlemail.com> wrote:
> > >
> > >> My preference is for trying to handle those common things via the
> tool,
> > >> i.e., rather than manually fixing apichanges.xml by hand, the point to
> > me
> > >> is to identify that that needs to be fixed, or XML files in general,
> and
> > >> add them to the generally problematic list at the top of the page.
> > >>
> > >> Gj
> > >>
> > >> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <ds...@bkwi.nl>
> > wrote:
> > >>
> > >>> Hi all,
> > >>>
> > >>> When you check a module for license headers and suspicious files
> (files
> > >>> that maybe were not owned by Oracle), make changes and do a Pull
> > Request
> > >>> (PR), the PR is checked by one or more committers. On the other hand,
> > when
> > >>> the person checking a module thinks no modifications are needed, no
> PR
> > is
> > >>> made and no code review is done (obviously). However, review of the
> > module
> > >>> check does not seem to take place either. What do we think of that?
> > >>>
> > >>> E.g., Geertjan is wotking his ass off, checking one module after
> > >>> another, but making very little PR's compared to the number of
> modules
> > he
> > >>> processes. When I review a couple of these modules, I think there
> > might be
> > >>> some oversights. E.g.
> > >>>
> > >>> api.intent has a Bundle.properties with an Oracle license header
> > >>>
> > >>> api.progress has a apichanges.xml with an Oracle license header
> > >>>
> > >>> api.progress.nb has aapichanges.xml with an Oracle license header
> > >>>
> > >>> I would expect those Oracle license headers to be removed or replaced
> > >>> with Apache license headers.
> > >>>
> > >>>
> > >>> How important is it to get the license headers right? Do we need a
> peer
> > >>> review on modules that seem okay? A second name in the name column on
> > the
> > >>> wiki page, confirming that no changes are needed?
> > >>>
> > >>> Please advise.
> > >>>
> > >>>
> > >>> /Dave
> > >>>
> > >>>
> > >>> @Geertjan: sorry to have used you as an example.
> > >>>
> > >>
> > >>
> > >
> >
>

Re: How to deal with module review without modifications

[Emilian Bold <em...@gmail.com>]

I mean, the whole phrase mentions this:

> -checked Rat report: everything has been relicensed to Apache, included
in 'central problems' list above, or excluded via Rat



--emi

On Wed, Sep 27, 2017 at 11:08 AM, Emilian Bold <em...@gmail.com>
wrote:

> I believe modules that still have .form files or other "Problems to be
> solved centrally" should be marked as "Done".
>
> The whole point is to look at everything else. Those problems will be
> solved... centrally.
>
>
> --emi
>
> On Wed, Sep 27, 2017 at 11:02 AM, Geertjan Wielenga <
> geertjan.wielenga@googlemail.com> wrote:
>
>> OK, I have marked all mine as "To do", since I am relying on the central
>> problems being handled via the tool, i.e., I think all "form" files, for
>> example, should be processed via the tool, rather than doing them
>> individually and manually per module.
>>
>> I think a peer review would be good, and yes, it is very important to get
>> the license headers right. A peer review column would be great, I think,
>> i.e., when a row is marked as Done someone could check that and provide
>> any
>> comments in the peer review column.
>>
>> Gj
>>
>> On Wed, Sep 27, 2017 at 12:32 AM, Geertjan Wielenga <
>> geertjan.wielenga@googlemail.com> wrote:
>>
>> > Actually, that means I shouldn't mark those modules as "Done" but as "To
>> > do".
>> >
>> > Gj
>> >
>> > On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
>> > geertjan.wielenga@googlemail.com> wrote:
>> >
>> >> My preference is for trying to handle those common things via the tool,
>> >> i.e., rather than manually fixing apichanges.xml by hand, the point to
>> me
>> >> is to identify that that needs to be fixed, or XML files in general,
>> and
>> >> add them to the generally problematic list at the top of the page.
>> >>
>> >> Gj
>> >>
>> >> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <ds...@bkwi.nl>
>> wrote:
>> >>
>> >>> Hi all,
>> >>>
>> >>> When you check a module for license headers and suspicious files
>> (files
>> >>> that maybe were not owned by Oracle), make changes and do a Pull
>> Request
>> >>> (PR), the PR is checked by one or more committers. On the other hand,
>> when
>> >>> the person checking a module thinks no modifications are needed, no
>> PR is
>> >>> made and no code review is done (obviously). However, review of the
>> module
>> >>> check does not seem to take place either. What do we think of that?
>> >>>
>> >>> E.g., Geertjan is wotking his ass off, checking one module after
>> >>> another, but making very little PR's compared to the number of
>> modules he
>> >>> processes. When I review a couple of these modules, I think there
>> might be
>> >>> some oversights. E.g.
>> >>>
>> >>> api.intent has a Bundle.properties with an Oracle license header
>> >>>
>> >>> api.progress has a apichanges.xml with an Oracle license header
>> >>>
>> >>> api.progress.nb has aapichanges.xml with an Oracle license header
>> >>>
>> >>> I would expect those Oracle license headers to be removed or replaced
>> >>> with Apache license headers.
>> >>>
>> >>>
>> >>> How important is it to get the license headers right? Do we need a
>> peer
>> >>> review on modules that seem okay? A second name in the name column on
>> the
>> >>> wiki page, confirming that no changes are needed?
>> >>>
>> >>> Please advise.
>> >>>
>> >>>
>> >>> /Dave
>> >>>
>> >>>
>> >>> @Geertjan: sorry to have used you as an example.
>> >>>
>> >>
>> >>
>> >
>>
>
>

Re: How to deal with module review without modifications

[Emilian Bold <em...@gmail.com>]

I believe modules that still have .form files or other "Problems to be
solved centrally" should be marked as "Done".

The whole point is to look at everything else. Those problems will be
solved... centrally.


--emi

On Wed, Sep 27, 2017 at 11:02 AM, Geertjan Wielenga <
geertjan.wielenga@googlemail.com> wrote:

> OK, I have marked all mine as "To do", since I am relying on the central
> problems being handled via the tool, i.e., I think all "form" files, for
> example, should be processed via the tool, rather than doing them
> individually and manually per module.
>
> I think a peer review would be good, and yes, it is very important to get
> the license headers right. A peer review column would be great, I think,
> i.e., when a row is marked as Done someone could check that and provide any
> comments in the peer review column.
>
> Gj
>
> On Wed, Sep 27, 2017 at 12:32 AM, Geertjan Wielenga <
> geertjan.wielenga@googlemail.com> wrote:
>
> > Actually, that means I shouldn't mark those modules as "Done" but as "To
> > do".
> >
> > Gj
> >
> > On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
> > geertjan.wielenga@googlemail.com> wrote:
> >
> >> My preference is for trying to handle those common things via the tool,
> >> i.e., rather than manually fixing apichanges.xml by hand, the point to
> me
> >> is to identify that that needs to be fixed, or XML files in general, and
> >> add them to the generally problematic list at the top of the page.
> >>
> >> Gj
> >>
> >> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <ds...@bkwi.nl>
> wrote:
> >>
> >>> Hi all,
> >>>
> >>> When you check a module for license headers and suspicious files (files
> >>> that maybe were not owned by Oracle), make changes and do a Pull
> Request
> >>> (PR), the PR is checked by one or more committers. On the other hand,
> when
> >>> the person checking a module thinks no modifications are needed, no PR
> is
> >>> made and no code review is done (obviously). However, review of the
> module
> >>> check does not seem to take place either. What do we think of that?
> >>>
> >>> E.g., Geertjan is wotking his ass off, checking one module after
> >>> another, but making very little PR's compared to the number of modules
> he
> >>> processes. When I review a couple of these modules, I think there
> might be
> >>> some oversights. E.g.
> >>>
> >>> api.intent has a Bundle.properties with an Oracle license header
> >>>
> >>> api.progress has a apichanges.xml with an Oracle license header
> >>>
> >>> api.progress.nb has aapichanges.xml with an Oracle license header
> >>>
> >>> I would expect those Oracle license headers to be removed or replaced
> >>> with Apache license headers.
> >>>
> >>>
> >>> How important is it to get the license headers right? Do we need a peer
> >>> review on modules that seem okay? A second name in the name column on
> the
> >>> wiki page, confirming that no changes are needed?
> >>>
> >>> Please advise.
> >>>
> >>>
> >>> /Dave
> >>>
> >>>
> >>> @Geertjan: sorry to have used you as an example.
> >>>
> >>
> >>
> >
>

Re: How to deal with module review without modifications

[Geertjan Wielenga <ge...@googlemail.com>]

OK, I have marked all mine as "To do", since I am relying on the central
problems being handled via the tool, i.e., I think all "form" files, for
example, should be processed via the tool, rather than doing them
individually and manually per module.

I think a peer review would be good, and yes, it is very important to get
the license headers right. A peer review column would be great, I think,
i.e., when a row is marked as Done someone could check that and provide any
comments in the peer review column.

Gj

On Wed, Sep 27, 2017 at 12:32 AM, Geertjan Wielenga <
geertjan.wielenga@googlemail.com> wrote:

> Actually, that means I shouldn't mark those modules as "Done" but as "To
> do".
>
> Gj
>
> On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
> geertjan.wielenga@googlemail.com> wrote:
>
>> My preference is for trying to handle those common things via the tool,
>> i.e., rather than manually fixing apichanges.xml by hand, the point to me
>> is to identify that that needs to be fixed, or XML files in general, and
>> add them to the generally problematic list at the top of the page.
>>
>> Gj
>>
>> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <ds...@bkwi.nl> wrote:
>>
>>> Hi all,
>>>
>>> When you check a module for license headers and suspicious files (files
>>> that maybe were not owned by Oracle), make changes and do a Pull Request
>>> (PR), the PR is checked by one or more committers. On the other hand, when
>>> the person checking a module thinks no modifications are needed, no PR is
>>> made and no code review is done (obviously). However, review of the module
>>> check does not seem to take place either. What do we think of that?
>>>
>>> E.g., Geertjan is wotking his ass off, checking one module after
>>> another, but making very little PR's compared to the number of modules he
>>> processes. When I review a couple of these modules, I think there might be
>>> some oversights. E.g.
>>>
>>> api.intent has a Bundle.properties with an Oracle license header
>>>
>>> api.progress has a apichanges.xml with an Oracle license header
>>>
>>> api.progress.nb has aapichanges.xml with an Oracle license header
>>>
>>> I would expect those Oracle license headers to be removed or replaced
>>> with Apache license headers.
>>>
>>>
>>> How important is it to get the license headers right? Do we need a peer
>>> review on modules that seem okay? A second name in the name column on the
>>> wiki page, confirming that no changes are needed?
>>>
>>> Please advise.
>>>
>>>
>>> /Dave
>>>
>>>
>>> @Geertjan: sorry to have used you as an example.
>>>
>>
>>
>

Re: How to deal with module review without modifications

[Geertjan Wielenga <ge...@googlemail.com>]

Actually, that means I shouldn't mark those modules as "Done" but as "To
do".

Gj

On Wed, Sep 27, 2017 at 12:27 AM, Geertjan Wielenga <
geertjan.wielenga@googlemail.com> wrote:

> My preference is for trying to handle those common things via the tool,
> i.e., rather than manually fixing apichanges.xml by hand, the point to me
> is to identify that that needs to be fixed, or XML files in general, and
> add them to the generally problematic list at the top of the page.
>
> Gj
>
> On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <ds...@bkwi.nl> wrote:
>
>> Hi all,
>>
>> When you check a module for license headers and suspicious files (files
>> that maybe were not owned by Oracle), make changes and do a Pull Request
>> (PR), the PR is checked by one or more committers. On the other hand, when
>> the person checking a module thinks no modifications are needed, no PR is
>> made and no code review is done (obviously). However, review of the module
>> check does not seem to take place either. What do we think of that?
>>
>> E.g., Geertjan is wotking his ass off, checking one module after another,
>> but making very little PR's compared to the number of modules he processes.
>> When I review a couple of these modules, I think there might be some
>> oversights. E.g.
>>
>> api.intent has a Bundle.properties with an Oracle license header
>>
>> api.progress has a apichanges.xml with an Oracle license header
>>
>> api.progress.nb has aapichanges.xml with an Oracle license header
>>
>> I would expect those Oracle license headers to be removed or replaced
>> with Apache license headers.
>>
>>
>> How important is it to get the license headers right? Do we need a peer
>> review on modules that seem okay? A second name in the name column on the
>> wiki page, confirming that no changes are needed?
>>
>> Please advise.
>>
>>
>> /Dave
>>
>>
>> @Geertjan: sorry to have used you as an example.
>>
>
>

Re: How to deal with module review without modifications

[Geertjan Wielenga <ge...@googlemail.com>]

My preference is for trying to handle those common things via the tool,
i.e., rather than manually fixing apichanges.xml by hand, the point to me
is to identify that that needs to be fixed, or XML files in general, and
add them to the generally problematic list at the top of the page.

Gj

On Wed, Sep 27, 2017 at 12:20 AM, Dave Schoorl <ds...@bkwi.nl> wrote:

> Hi all,
>
> When you check a module for license headers and suspicious files (files
> that maybe were not owned by Oracle), make changes and do a Pull Request
> (PR), the PR is checked by one or more committers. On the other hand, when
> the person checking a module thinks no modifications are needed, no PR is
> made and no code review is done (obviously). However, review of the module
> check does not seem to take place either. What do we think of that?
>
> E.g., Geertjan is wotking his ass off, checking one module after another,
> but making very little PR's compared to the number of modules he processes.
> When I review a couple of these modules, I think there might be some
> oversights. E.g.
>
> api.intent has a Bundle.properties with an Oracle license header
>
> api.progress has a apichanges.xml with an Oracle license header
>
> api.progress.nb has aapichanges.xml with an Oracle license header
>
> I would expect those Oracle license headers to be removed or replaced with
> Apache license headers.
>
>
> How important is it to get the license headers right? Do we need a peer
> review on modules that seem okay? A second name in the name column on the
> wiki page, confirming that no changes are needed?
>
> Please advise.
>
>
> /Dave
>
>
> @Geertjan: sorry to have used you as an example.
>