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.

Sign up for GitHub

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

Jump to bottom

Move addFields to ParseUtils #10394

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Move addFields to ParseUtils #10394

wants to merge 2 commits into from

Conversation

9999years
Copy link
Collaborator

Split off of #10292.

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@9999years
Add type and documentation to toSavedConfig
23e0f44 
This helper function was challenging to understand, so I added
an explicit type signature and documentation comments to it.
@9999years
Move addFields to ParseUtils, add aliasField helper
1a20d92 
`addFields` should be in `ParseUtils` with the rest of the field
helpers.
This was referenced Sep 27, 2024
Open
Open
@9999years 9999years added the merge me Tell Mergify Bot to merge label Sep 28, 2024
@9999years 9999years marked this pull request as ready for review September 28, 2024 01:17
@mpickering
Copy link
Collaborator

This patch doesn't make sense without a patch which also uses aliasFields. I find it confusing it's a separate MR rather than included in the one which actually needs it.

@9999years
Copy link
Collaborator Author

@mpickering I felt that keeping independent changes in independent PRs makes each PR smaller and easier to review, but I've combined this change into #10393 if you prefer that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@geekosaur geekosaur geekosaur approved these changes

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
merge me Tell Mergify Bot to merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@9999years @mpickering @geekosaur