[GRASS-dev] GitHub: how to fwd pull requests to this list?

GRASS GRASS developer
1

Hi,

does anyone know how to how to forward GitHub pull requests to this list?

They tend to get lost:
https://github.com/OSGeo/grass/pulls
https://github.com/OSGeo/grass-addons/pulls

thanks
Markus

2

And, maybe, this question is related. I don’t receive any notifications for my own PRs. PRs from other devs are OK. Is that normal?

Thanks,
Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

3

Markus,

I think I have a workaround. In the worst case scenario, if GitHub doesn’t support notification forwarding, we could create a dummy GitHub account just for notifications and set its email address to grass-dev@lists.osgeo.org.

Just my thoughts.

Regards,
Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

4

Hi Huidae,

Huidae Cho <grass4u@gmail.com> schrieb am Fr., 12. Juli 2019, 05:40:

And, maybe, this question is related. I don’t receive any notifications for my own PRs. PRs from other devs are OK. Is that normal?

I have the same problem and don’t like it even if that might be “normal”.

Best
Markus

5

Hi,

On Fri, Jul 12, 2019 at 6:12 AM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

I think I have a workaround. In the worst case scenario, if GitHub doesn't support notification forwarding, we could create a dummy GitHub account just for notifications and set its email address to grass-dev@lists.osgeo.org.

Well, we do have this list:

https://lists.osgeo.org/pipermail/grass-commit/

which contains everything from GitHub (I had set it up like this).

But I would like to see _only_ the PRs also here and not clutter this
list with the other GH emails.
Not sure how to do that...

Best
Markus

6

Markus,

I don’t think that list has PR notifications. That’s only for actual merges, not everything, if I’m not wrong.

Best,
Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

7

The biggest question is – do we need PR notifications here. I would
vote for no. Better let's keep discussions in one place.

And no, PRs don't get lost. They all are on GitHub. If a PR was lost,
that is a bug in GitHub and needs to be addressed ASAP.

When we moved to the new workflow, those with rights to merge PRs
(implicitly) agreed to go over PR list on a (semi)regular basis and
merge them if they are good.

Have a nice day,
Māris.

piektd., 2019. g. 12. jūl., plkst. 17:10 — lietotājs Huidae Cho
(<grass4u@gmail.com>) rakstīja:

Markus,

I don't think that list has PR notifications. That's only for actual merges, not everything, if I'm not wrong.

Best,
Huidae

On Fri, Jul 12, 2019 at 9:58 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Fri, Jul 12, 2019 at 6:12 AM Huidae Cho <grass4u@gmail.com> wrote:
>
> Markus,
>
> I think I have a workaround. In the worst case scenario, if GitHub doesn't support notification forwarding, we could create a dummy GitHub account just for notifications and set its email address to grass-dev@lists.osgeo.org.

Well, we do have this list:

https://lists.osgeo.org/pipermail/grass-commit/

which contains everything from GitHub (I had set it up like this).

But I would like to see _only_ the PRs also here and not clutter this
list with the other GH emails.
Not sure how to do that...

Best
Markus

--
Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team
_______________________________________________
grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

8

On Fri, Jul 12, 2019 at 12:50 PM Maris Nartiss <maris.gis@gmail.com> wrote:

The biggest question is – do we need PR notifications here. I would
vote for no. Better let’s keep discussions in one place.

I agree, and I get the notifications:
https://help.github.com/en/articles/watching-and-unwatching-repositories

And no, PRs don’t get lost. They all are on GitHub. If a PR was lost,
that is a bug in GitHub and needs to be addressed ASAP.

When we moved to the new workflow, those with rights to merge PRs
(implicitly) agreed to go over PR list on a (semi)regular basis and
merge them if they are good.

Have a nice day,
Māris.

piektd., 2019. g. 12. jūl., plkst. 17:10 — lietotājs Huidae Cho
(<grass4u@gmail.com>) rakstīja:

Markus,

I don’t think that list has PR notifications. That’s only for actual merges, not everything, if I’m not wrong.

Best,
Huidae

On Fri, Jul 12, 2019 at 9:58 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi,

On Fri, Jul 12, 2019 at 6:12 AM Huidae Cho <grass4u@gmail.com> wrote:

Markus,

I think I have a workaround. In the worst case scenario, if GitHub doesn’t support notification forwarding, we could create a dummy GitHub account just for notifications and set its email address to grass-dev@lists.osgeo.org.

Well, we do have this list:

https://lists.osgeo.org/pipermail/grass-commit/

which contains everything from GitHub (I had set it up like this).

But I would like to see only the PRs also here and not clutter this
list with the other GH emails.
Not sure how to do that…

Best
Markus

–
Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev

9

Hi

Anna Petrášová <kratochanna@gmail.com> schrieb am Mo., 15. Juli 2019, 05:57:

On Fri, Jul 12, 2019 at 12:50 PM Maris Nartiss <maris.gis@gmail.com> wrote:

The biggest question is – do we need PR notifications here. I would
vote for no. Better let’s keep discussions in one place.

I agree, and I get the notifications:
https://help.github.com/en/articles/watching-and-unwatching-repositories

Then please all promise to subscribe to the repo notifications and look at them :slight_smile:

Thanks
Markus

PS: with “lost” I meant that nobody cares/d. Perhaps since many didn’t subscribe to notifications yet…

10

Hi,

The oldest PR is 2 months old. Using git for collaboration is very different from svn, I believe. Unlike with svn, you have to “actively” merge these PRs into your branch for testing (e.g., features interesting to you?) and wait for an approval from reviewers (?). Why did it not happen to these old PRs? Now, a less bigger question is no one cares about them and who is supposed to be responsible for reviewing and approving them?

For example, https://github.com/OSGeo/grass/pull/52 fixes a very obvious formatting issue, which still needs to be approved? Tried to approve it myself, but self-nominating myself as a reviewer is pending… Hmm… Can I even merge it? The merge button seems to be activated. Confused…

Best,
Huidae

···

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

11

On Mon, Jul 15, 2019 at 12:07 PM Huidae Cho <grass4u@gmail.com> wrote:

Hi,

The oldest PR is 2 months old. Using git for collaboration is very different from svn, I believe. Unlike with svn, you have to “actively” merge these PRs into your branch for testing (e.g., features interesting to you?) and wait for an approval from reviewers (?). Why did it not happen to these old PRs? Now, a less bigger question is no one cares about them and who is supposed to be responsible for reviewing and approving them?

Patches in svn tickets sometimes got old too. When no one cares (or has time), then it doesn’t matter if we use svn or github. I would say PRs on github are easier to find and track than what we used before, although seem harder to test, but that’s what we signed up for.
If you want someone’s review, you can request it. If not, you can merge it yourself.

For example, https://github.com/OSGeo/grass/pull/52 fixes a very obvious formatting issue, which still needs to be approved? Tried to approve it myself, but self-nominating myself as a reviewer is pending… Hmm… Can I even merge it? The merge button seems to be activated. Confused…

I actually asked in the related ticket for testing because I couldn’t reproduce the problem (the formatting influences unicode/bytes). I didn’t merge it because there is always a chance more changes might be needed and then you can update the PR and squash the commits and keep it nice and clean.

Anna

Best,
Huidae

On Mon, Jul 15, 2019 at 6:53 AM Markus Neteler <neteler@osgeo.org> wrote:

Hi

Anna Petrášová <kratochanna@gmail.com> schrieb am Mo., 15. Juli 2019, 05:57:

On Fri, Jul 12, 2019 at 12:50 PM Maris Nartiss <maris.gis@gmail.com> wrote:

The biggest question is – do we need PR notifications here. I would
vote for no. Better let’s keep discussions in one place.

I agree, and I get the notifications:
https://help.github.com/en/articles/watching-and-unwatching-repositories

Then please all promise to subscribe to the repo notifications and look at them :slight_smile:

Thanks
Markus

PS: with “lost” I meant that nobody cares/d. Perhaps since many didn’t subscribe to notifications yet…

–

Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team

12

Patches in svn tickets sometimes got old too. When no one cares (or has
time), then it doesn't matter if we use svn or github. I would say PRs on
github are easier to find and track than what we used before,

although seem
harder to test, but that's what we signed up for.

Given a pull request of number ID, that's just a 2-liner:

git fetch origin pull/{ID}/head:pr-{ID}
git checkout pr-{ID}

(more verbose explanations at
https://help.github.com/en/articles/checking-out-pull-requests-locally )

But actually, you should rarely need to test a pull request locally if you
take care of checking that the pull request which adds new functionality comes
with associated tests. And if it is a bugfix, hopefully your test suite should
already test them modified code. If not, then that's the opportunity for the
pull request to add that new test.

Pull request do work well with a strong test suite.

Looks like https://github.com/OSGeo/grass/blob/master/.travis/linux.script.sh
is missing a "make check" target :slight_smile:

Even

--
Spatialys - Geospatial professional services
http://www.spatialys.com

13

Markus Neteler wrote

Hi

Anna Petrášová &lt;

kratochanna@

&gt; schrieb am Mo., 15. Juli 2019, 05:57:

On Fri, Jul 12, 2019 at 12:50 PM Maris Nartiss &lt;

maris.gis@

&gt;

wrote:

The biggest question is – do we need PR notifications here. I would
vote for no. Better let's keep discussions in one place.

I agree, and I get the notifications:
https://help.github.com/en/articles/watching-and-unwatching-repositories

Then please all promise to subscribe to the repo notifications and look at
them :slight_smile:

already subscribed.

somekind related

in https://trac.osgeo.org/grass/wiki/HowToGit there is mentioned:

####
# <make local source code changes>
vim ...

# list local changes
git status
git add file1.c file2.py ...
git commit -m 'my change with reasonable explanation...'
####

at least here in the windows world of git I had to use

####
git commit -am 'my change with reasonable explanation...'
####

to add also the -a switch that anything happend .... :smiley:

otherwise I get the message:

####
$ git commit -m 'add missing dependencies libgraphite2.dll and libpcre-1.dll
- see https://trac.osgeo.org/grass/ticket/3870’
On branch wingrass_fixes
Changes not staged for commit:
        modified: mswindows/osgeo4w/package.sh

no changes added to commit
####

-----
best regards
Helmut
--
Sent from: http://osgeo-org.1560.x6.nabble.com/Grass-Dev-f3991897.html

14

Are you sure you did git add? -a just adds “all changes”. I remember add & -m only worked for me on Windows (msys2 git).

Regards,
Huidae

–
Huidae Cho, Ph.D., GISP, PE (MD), CFM, M.ASCE
Open Source GIS Developer, GRASS GIS Development Team
https://idea.isnew.info
Sent from my phone

On Mon, Jul 15, 2019, 7:04 PM Helmut Kudrnovsky <hellik@web.de> wrote:

Markus Neteler wrote

Hi

Anna Petrášová <

kratochanna@

> schrieb am Mo., 15. Juli 2019, 05:57:

On Fri, Jul 12, 2019 at 12:50 PM Maris Nartiss <

maris.gis@

>

wrote:

The biggest question is – do we need PR notifications here. I would
vote for no. Better let’s keep discussions in one place.

I agree, and I get the notifications:
https://help.github.com/en/articles/watching-and-unwatching-repositories

Then please all promise to subscribe to the repo notifications and look at
them :slight_smile:

already subscribed.

somekind related

in https://trac.osgeo.org/grass/wiki/HowToGit there is mentioned:

vim …

list local changes

git status
git add file1.c file2.py …
git commit -m ‘my change with reasonable explanation…’

at least here in the windows world of git I had to use

git commit -am ‘my change with reasonable explanation…’

to add also the -a switch that anything happend … :smiley:

otherwise I get the message:

$ git commit -m 'add missing dependencies libgraphite2.dll and libpcre-1.dll

no changes added to commit

best regards
Helmut

Sent from: http://osgeo-org.1560.x6.nabble.com/Grass-Dev-f3991897.html

grass-dev mailing list
grass-dev@lists.osgeo.org
https://lists.osgeo.org/mailman/listinfo/grass-dev