Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Assorted fixes: Makefile, tests, gie enhancement, Travis-CI simplification #1088

Merged
merged 4 commits into from Aug 13, 2018

Conversation

@rouault
Copy link
Member

@rouault rouault commented Aug 11, 2018

No description provided.

Copy link
Member

@kbevers kbevers left a comment

The new gie keyword should be documented in docs/source/apps/gie.rst.

It doesn't seem to be possible to require more than one grid, is that true? I think it would be useful if you were able to require several grids, since that is needed in some transformations.

@@ -24,7 +24,7 @@ EXTRA_DIST = GL27 nad.lst proj_def.dat nad27 nad83 pj_out27.dist pj_out83.dist t
esri.extra other.extra \
CH IGNF testIGNF proj_outIGNF.dist \
ITRF2000 ITRF2008 ITRF2014 \
makefile.vc CMakeLists.txt tests/test_nodata.gtx
makefile.vc CMakeLists.txt tests/test_nodata.gtx null.lla

This comment has been minimized.

@kbevers

kbevers Aug 12, 2018
Member

What's the reasoning behind including it here? The null grid is included in the grid distributions.

This comment has been minimized.

@rouault

rouault Aug 12, 2018
Author Member

The reasoning is that null.lla is in git, and if we don't had it in EXTRA_DIST it is not part of the generated tarball by make dist. And thus if someone use the tarball and does "make check" without downloading the grids, it won't get the null grid (since 'make' in nad create null from null.lla) and that causes a number of test failures. I think it is reasonable to have the tiny null grid generated by proj itself

This comment has been minimized.

@kbevers

kbevers Aug 12, 2018
Member

All right. I agree, it is reasonable to include it.

@@ -516,6 +517,20 @@ static int ignore (const char *args) {
return 0;
}

static int require_grid (const char *args) {

This comment has been minimized.

@kbevers

kbevers Aug 12, 2018
Member

This is a great idea! Previously we've handled missing grids by adding ignore pjd_failed_to_load_grid. I think those would be handled better by this new require_grid keyword.

@rouault
Copy link
Member Author

@rouault rouault commented Aug 12, 2018

It doesn't seem to be possible to require more than one grid, is that true?

It is possible. You just to have repeat the require_grid keyword several times

@rouault rouault merged commit 88c2088 into OSGeo:master Aug 13, 2018
4 checks passed
4 checks passed
@travis-ci
Travis CI - Pull Request Build Passed
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.03%) to 77.748%
Details
@kbevers kbevers added this to the 5.2.0 milestone Aug 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants