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

Make vector arg to fill-multi/fill-human-multi fill in order #652

Merged

Conversation

dgr
Copy link
Contributor

@dgr dgr commented Aug 29, 2024

Previously, if the user supplied a vector argument to fill-multi or fill-human-multi, Etaoin would convert the vector to a map, then convert the map to a sequence of MapEntry pairs, and use those to fill multiple fields. Unfortunately, this threw away all the ordering information in the vector. This change processes the vector as a sequence of partitioned pairs and thereby keeps the order intact.

Closes #649

Please complete and include the following checklist:

  • I have read CONTRIBUTING and the Etaoin Developer Guide.

  • This PR corresponds to an issue that the Etaoin maintainers have agreed to address.

  • This PR contains test(s) to protect against future regressions

  • I have updated CHANGELOG.adoc with a description of the addressed issue.

Previously, if the user supplied a vector argument to fill-multi or
fill-human-multi, Etaoin would convert the vector to a map, then
convert the map to a sequence of MapEntry pairs, and use those to fill
multiple fields. Unfortunately, this threw away all the ordering
information in the vector. This change processes the vector as a
sequence of partitioned pairs and thereby keeps the order intact.
Copy link
Collaborator

@lread lread left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only need a small fix to changelog!

CHANGELOG.adoc Show resolved Hide resolved
Copy link
Collaborator

@lread lread left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More changelog fixes?

CHANGELOG.adoc Outdated
Comment on lines 30 to 32
** {issue}559[#559]: Deprecate use of `:active` with `query` and other APIs that use `query` under the hood. ({person}dgr[@dgr])
** {issue}642[#642]: Add `driver-type` to retrieve driver type keyword. ({person}dgr[@dgr])
** {issue}644[#644]: Deprecate `when-predicate` and `when-not-predicate` macros. See docstrings for guidance on alternatives. These will may be removed from the API at the next release that contains breaking changes. ({person}dgr[@dgr])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dave, are any of these strictly doc changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding driver-type is not just a doc change. The others largely are (in the sense that adding ^:deprecated is a doc change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me go through and check each one carefully and submit a change set to the PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, try that. I moved to the correct section and put them in numerical order.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry @dgr, I'm going to make you suffer one more time!
I think deprecating an fn is more than a doc change and, therefore, shouldn't be under doc changes. (If there are no doc changes left, feel free to delete the doc item in the list.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

@lread lread merged commit 5bbe8c7 into clj-commons:master Aug 31, 2024
49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Modify fill-multi and fill-human-multi so that q-text vector type fills fields in vector order
2 participants