XLSForm / pyxform

A Python package to create XForms for ODK Collect.
BSD 2-Clause "Simplified" License
77 stars 134 forks source link

Add rows to parameters column for text type #667

Closed grzesiek2010 closed 7 months ago

grzesiek2010 commented 7 months ago

Closes #616

Why is this the best possible solution? Were any other approaches considered?

The solution is consistent with how we handle other parameters in other question types. In My previous pull request, I added a new parameter for image questions (see https://github.com/XLSForm/pyxform/pull/659) and this one solves the problem in the same way. I haven't considered any other approaches. I know there is a plan to Create a framework for adding parameters and then the solution would be different but for now we should follow the existing patterns.

What are the regression risks?

This is not a risky change. It's isolated and touches only reading parameters used in text questions.

Does this change require updates to documentation? If so, please file an issue here and include the link below.

Yes: https://github.com/XLSForm/xlsform.github.io/issues/254

Before submitting this PR, please make sure you have:

lindsay-stevens commented 7 months ago

Nothing to discuss here.

I mean, OK, but that's a bit flippant. You could say whether or not you followed the existing patterns for parameters (and why), whether or not you searched for other issues that might overlap with the linked issue, whether or not you tested the XForm output with Collect and/or Enketo, etc.

I often use PR descriptions, issues, and commit messages to do code archaeology to understand why things are a certain way, so adding some detail is worth the effort even if it seems obvious now.

grzesiek2010 commented 7 months ago

I mean, OK, but that's a bit flippant. You could say whether or not you followed the existing patterns for parameters (and why), whether or not you searched for other issues that might overlap with the linked issue, whether or not you tested the XForm output with Collect and/or Enketo, etc.

I often use PR descriptions, issues, and commit messages to do code archaeology to understand why things are a certain way, so adding some detail is worth the effort even if it seems obvious now.

Yeah, that makes sense. Even if something is obvious adding a short description might be valuable for other devs.

lindsay-stevens commented 7 months ago

@grzesiek2010 thanks!