-
Notifications
You must be signed in to change notification settings - Fork 3
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
Mcpods 6515 update apis #97
Conversation
…6515-update--new # Conflicts: # here-naksha-app-service/src/test/java/com/here/naksha/app/service/NakshaAppTest.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Submitting first round of review feedback. Will take a look again, when all changes are addressed
here-naksha-app-service/src/test/java/com/here/naksha/app/service/NakshaAppTest.java
Show resolved
Hide resolved
...pp-service/src/test/resources/unit_test_data/TC0500_updateFeatures/response_no_geometry.json
Show resolved
Hide resolved
here-naksha-app-service/src/test/java/com/here/naksha/app/service/NakshaAppTest.java
Show resolved
Hide resolved
here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/util/storage/RequestHelper.java
Outdated
Show resolved
Hide resolved
…tToXyzCollectionResponse
...ha-app-service/src/main/java/com/here/naksha/app/service/http/tasks/WriteFeatureApiTask.java
Outdated
Show resolved
Hide resolved
...ha-app-service/src/main/java/com/here/naksha/app/service/http/tasks/WriteFeatureApiTask.java
Outdated
Show resolved
Hide resolved
here-naksha-app-service/src/test/java/com/here/naksha/app/service/UpdateFeatureTestHelper.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API and the test coverage seem quite aligned now. Still some minor comments, that I would like to review again.
...ha-app-service/src/main/java/com/here/naksha/app/service/http/tasks/WriteFeatureApiTask.java
Outdated
Show resolved
Hide resolved
...ha-app-service/src/main/java/com/here/naksha/app/service/http/tasks/WriteFeatureApiTask.java
Outdated
Show resolved
Hide resolved
here-naksha-app-service/src/main/resources/swagger/openapi.yaml
Outdated
Show resolved
Hide resolved
here-naksha-app-service/src/test/java/com/here/naksha/app/service/UpdateFeatureTestHelper.java
Show resolved
Hide resolved
here-naksha-app-service/src/test/java/com/here/naksha/app/service/UpdateFeatureTestHelper.java
Outdated
Show resolved
Hide resolved
here-naksha-app-service/src/test/java/com/here/naksha/app/service/UpdateFeatureTestHelper.java
Show resolved
Hide resolved
here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/util/storage/RequestHelper.java
Outdated
Show resolved
Hide resolved
…6515-update--new # Conflicts: # here-naksha-lib-core/src/main/java/com/here/naksha/lib/core/util/storage/ResultHelper.java # here-naksha-lib-hub/src/main/java/com/here/naksha/lib/hub/mock/MockWriteResult.java
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR can go ahead, once review comments are addressed.
here-naksha-app-service/src/main/resources/swagger/openapi.yaml
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Amaneusz - is it right to assume that we can now support geometry verification in the API responses? Or it still requires some more alignment?
If support already available, then @gunplar I recommend we start adding geometry based verification again in the API responses. If not, then we can just create a ticket for now and can pick it up again soon.
Either ways, this PR can still go ahead as long as other comments are addressed. Geometry verification can be picked up as separate item.
No description provided.