Skip to content
This repository has been archived by the owner on Nov 29, 2023. It is now read-only.

Fix: multiple "static" itemset datalists in the same repeat #995

Merged
merged 3 commits into from
Aug 17, 2023

Conversation

eyelidlessness
Copy link
Contributor

This fixes a case identified by the recent pyxform change to always generate secondary instances for selects. Hopefully the in-code commentary is enough elaboration on the nature of the bug and its fix.

contributing factors are spread across multiple files/modules, across
multiple projects

Such information is probably better suited for, perhaps, a knowledge base
Copy link
Contributor

Choose a reason for hiding this comment

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

Have you seen this done in an effective way? From my experience, inline comments are about as good as it gets, especially with keeping things up to date. The ideal is of course not to have such unexpected interactions so that commentary isn't needed at all but for now this seems like the best spot so this paragraph could be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've seen effective use of internal documentation tools for e.g. cross-cutting concerns where sufficiently embraced. I'm not sure how effective it'd be with the current structure or how we'd best utilize it, but it's worth thinking about. For now, I'm fine with removing the paragraph.

I will say I'm concerned that much of this commentary well be more valuable upfront than over time, as its connection to the behaviors it describes links to the pertinent code rather than from it. But I'm not sure how we'd best solve that either.

Copy link
Contributor

@lognaturel lognaturel left a comment

Choose a reason for hiding this comment

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

I have verified that this also works with the user-provided full form.

@eyelidlessness I'll let you decide whether you want to cut out the paragraph about alternative places for the commentary.

@lognaturel lognaturel merged commit 6bebe25 into master Aug 17, 2023
4 checks passed
@lognaturel lognaturel deleted the fix/itemset-template-wrong-siblings branch August 17, 2023 21:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants