Added get_callee() which determines the right routine of the return values of get_callees()

[schreiberx]

It does what's in the title.

[arporter]

Thanks @schreiberx, unfortunately, linting is still failing.

In case it helps, I have the following pre-push hook setup:

$ cat PSyclone/.git/hooks/pre-push
#!/usr/bin/sh

# An example hook script to verify what is about to be pushed.  Called by "git
# push" after it has checked the remote status, but before anything has been
# pushed.  If this script exits with a non-zero status nothing will be pushed.
#
# This hook is called with the following parameters:
#
# $1 -- Name of the remote to which the push is being done
# $2 -- URL to which the push is being done
#
# If pushing without using a named remote those arguments will be equal.
#
# Information about the commits which are being pushed is supplied as lines to
# the standard input in the form:
#
#   <local ref> <local oid> <remote ref> <remote oid>
#
# This script ensures that the whole of PSyclone is linted successfully
# before the push is executed.

remote="$1"
url="$2"

if ! command -v flake8; then
    echo "WARNING: source not linted because flake8 unavailable"
    exit 0
fi

flake8=$(flake8 src/psyclone)
if (( flake8 == 1 )); then
    echo "Linting failed"
else
    echo "Linting succeeded"
fi
exit $flake8

[schreiberx]

Hi Andy, Thanks for this script.
I think that all these helper scripts should be part of psyclone to quickly run them over the source code. I added this script in the newest pushed version in bin_git/run_flake8.sh.
Cheers,
Martin

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.88%. Comparing base (6074d9c) to head (b71f714).
Report is 33 commits behind head on master.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #2775   +/-   ##
=======================================
  Coverage   99.88%   99.88%           
=======================================
  Files         357      357           
  Lines       49652    49708   +56     
=======================================
+ Hits        49596    49652   +56     
  Misses         56       56           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[arporter]

@schreiberx please could you hold off making further changes until I pass it back to you? It's hard to review if code changes underneath me while I'm doing it :-)

[schreiberx]

These changes are only addressing increasing the coverage to 100% since the codecov bot complained with "Please review."
I'll keep my hands off for now :-)

[arporter]

Thanks very much for this Martin. I like the new routine making use of the existing functionality, that seems sensible. I did struggle to get my head around it so you'll see I've requested that some comments be tweaked or added.
As I say in the review, I think a little work on the frontend for those situations where we hit OPTIONAL could really help here.
The only thing I'm really not sure of is the passing around of the list of integers. That seems a bit odd so I'd like to understand why you do that.
I've not looked at the test suite yet (but thanks for addressing the missed lines).

[arporter]

Back to @schreiberx :-)

[schreiberx]

Back to @arporter :-)

[schreiberx]

Back to @arporter :-)

[schreiberx]

"codecov/patch — 100.00% of diff hit" => back to @arporter :-)

[arporter]

Thanks very much Martin. Pretty much there now bar some tidying.
Also, please could you extend the developer guide to mention the new run_flake8.sh script:
Please edit doc/developer_guide/working_practises.rst and add a new section called "Linting" immediately after the one on "Continuous Integration". This just needs to say that all code is linted by flake8 and must be free from errors to be merged with master. This check is performed automatically by the CI and it can be useful to setup a git hook to do this check before pushing to the repository. An example script is provided in ....

Please could you then refer to this section from the second bullet point in Continuous Integration.

I'll check the integration tests once #2697 is on (as that fixes the NEMO ones).

[schreiberx]

There's the documentation of flake8:
https://github.com/stfc/PSyclone/blob/martin_nemo/doc/developer_guide/working_practises.rst#flake8

Back to @arporter

[schreiberx]

PS: I wasn't able to reply to "Use name 'd' which doesn't exist", but did the changes in the comments. Thanks for spotting this error.

[schreiberx]

I don't know what's causing this build problem of build-documentation:

/home/runner/work/PSyclone/PSyclone/doc/user_guide/index.rst:115: WARNING: toctree contains reference to document 'zz_bibliography' that doesn't have a title: no link will be generated [toc.no_title]

I'm not able to reproduce it on my computer.

[arporter]

Thanks very much Martin.
This is now ready to be merged :-)
I'll bring it up to master and tweak the two bits of text that I've mentioned in the review. Then I'll run the integration tests. Then I shall merge it...

[arporter]

Integration tests have finally finished and were green. Will proceed to merge.