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

Remove Add Batch Output from api #78

Merged
merged 6 commits into from
Mar 17, 2022
Merged

Conversation

pyrocumulus
Copy link
Owner

PVOutput removed their addbatchoutput.jsp entrypoint and made batching of outputs a donation only option within the (originally) single add output entrypoint.

We need to remove our AddBatchOutput method and request and we need to change the AddOutput to support multiple outputs in one request. Also update the documentation on our end, to make clear that this is a donation only feature.

See new documentation:
https://pvoutput.org/help/api_specification.html#batching-outputs

When done, this will close #75

@pyrocumulus pyrocumulus added the api breaking change This issue or pull request will result in a breaking change label Jul 23, 2021
@pyrocumulus pyrocumulus added this to the 1.0.0 milestone Jul 23, 2021
@pyrocumulus pyrocumulus self-assigned this Jul 23, 2021
- Removed inheritance for OutputPost / BatchOutputPost; merged all properties
- Removed inheritance for their respective builders
- Renamed all occurences of BatchOutput to Outputs
- Added new properties to IOutputPost as per documentation
- Implemented those properties in the AddOutput request
- Removed old AddBatchOutputRequest and added AddOutputsRequest with a baseline for operation

TODO:
Unit tests are currently disabled
New properties can not yet be set through the builders
@codecov
Copy link

codecov bot commented Nov 9, 2021

Codecov Report

Merging #78 (8372359) into develop (eb4a9b4) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff            @@
##           develop      #78   +/-   ##
========================================
  Coverage    99.49%   99.50%           
========================================
  Files          120      116    -4     
  Lines         2188     2212   +24     
  Branches       140      144    +4     
========================================
+ Hits          2177     2201   +24     
  Misses           4        4           
  Partials         7        7           

Added new properties to OutputPostBuilder
Rewritten all batch related unit tests
Added more tests for all the new properties
Removed unnecessary analyses suppressions
Fixed naming convention violations
Tidied up the explicit / var usaged
@pyrocumulus pyrocumulus merged commit 559377f into develop Mar 17, 2022
@pyrocumulus pyrocumulus deleted the batch-outputs-change branch March 17, 2022 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api breaking change This issue or pull request will result in a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

pvoutput deprecated addbatchoutput
1 participant