SW-1971 Add endpoint to transfer seeds to nursery #705
Conversation
`POST /api/v2/seedbank/accessions/{id}/transfers/nursery` will withdraw seeds from
an accession and create a new seedling batch.
|
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
| val date: LocalDate, | ||
| val destinationFacilityId: FacilityId, | ||
| @JsonSetter(nulls = Nulls.FAIL) | ||
| @Min(0) // |
There was a problem hiding this comment.
should the // be removed?
There was a problem hiding this comment.
It's there to stop ktfmt from putting the annotations and the val on a single line, which I thought made the class harder to read since there's a mix of annotated and non-annotated properties. I initially tried adding an explanation after the // but that also added visual clutter.
The other option would be to put the annotated properties all together and let ktfmt do its default thing. I kind of like having properties in alphabetical order but maybe it's a bad tradeoff here.
There was a problem hiding this comment.
Though maybe if I did // line-break or something, that might look okay and look less like a mistake; I'll try that.
There was a problem hiding this comment.
Got it, this is fine, alphabetical order is easier to read/search.
Doe ktfmt have a config option to override default behavior?
There was a problem hiding this comment.
ktfmt is deliberately not configurable. It's of the class of code formatters like gofmt and Black that take an approach of, "The point of an automated formatter is to stop people from wasting time arguing about formatting choices, and adding a bunch of fine-grained settings would cause people to waste time fussing with the settings instead." So its only configuration option is to control which of two Kotlin style guides it follows.
When I've reported formatting problems, they've often agreed that it isn't acting as advertised and fixed the problems. I just filed an issue earlier today, in fact: facebook/ktfmt#360
Splitting on annotations has been an open request for a while and it sounds like the authors are on the fence about whether it's a good thing to change: facebook/ktfmt#226
There was a problem hiding this comment.
Thanks for the info. Just so you know, this PR is now mentioned in the FB issues :)
POST /api/v2/seedbank/accessions/{id}/transfers/nurserywill withdraw seeds froman accession and create a new seedling batch.