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

Read and write large negative longitudes #2197

Merged
merged 2 commits into from Jul 10, 2018

Conversation

Projects
None yet
2 participants
@calum-chamberlain
Contributor

calum-chamberlain commented Jul 9, 2018

What does this PR do?

Fixes a bug identified in #1991. Longitudes between -100 and -180 were not read or written properly. This is also fixed upstream in #2195, which may result in a merge issue when maintenance is merged into master... Both have the same tests, but master has updated string formatting which maintenance does not have.

PR Checklist

  • Correct base branch selected? master for new features, maintenance_... for bug fixes
  • This PR is not directly related to an existing issue (which has no PR yet).
    - [ ] If the PR is making changes to documentation, docs pages can be built automatically.
    Just remove the space in the following string after the + sign: "+ DOCS"

    - [ ] If any network modules should be tested for the PR, add them as a comma separated list
    (e.g. clients.fdsn,clients.arclink) after the colon in the following magic string: "+TESTS:"
    (you can also add "ALL" to just simply run all tests across all modules)
  • All tests still pass.
  • Any new features or fixed regressions are be covered via new tests.
    - [ ] Any new or changed features have are fully documented.
  • Significant changes have been added to CHANGELOG.txt .
    - [ ] First time contributors have added your name to CONTRIBUTORS.txt .

@calum-chamberlain calum-chamberlain added this to the 1.1.1 milestone Jul 9, 2018

@megies

megies approved these changes Jul 10, 2018

CI passing and same as part of #2195 so 👍

@megies megies merged commit e594292 into maintenance_1.1.x Jul 10, 2018

5 of 6 checks passed

docker-testbot docker testbot results not available yet
ci/circleci Your tests passed on CircleCI!
Details
codecov/patch 100% of diff hit (target 90%)
Details
codecov/project 88.09% (+1.33%) compared to b4b8199
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@megies megies deleted the negative-long-nordic-patch branch Jul 10, 2018

@megies

This comment has been minimized.

Member

megies commented Jul 10, 2018

Thanks @calum-chamberlain! I had to resolve the expected conflicts when merging maintenance into master due to this one. No big deal, basically just dropping the changes on maintenance again in favor of the newer ones you did on master earlier (this sounds like time travel :0 -- and moving the mention in changelog around). You can check the merge commit here: a77b384

@calum-chamberlain

This comment has been minimized.

Contributor

calum-chamberlain commented Jul 10, 2018

Ideal, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment