Skip to content

Refactor Check Fields into helper function#246

Open
InvincibleRMC wants to merge 2 commits intoros2:rollingfrom
InvincibleRMC:refactor-check-fields
Open

Refactor Check Fields into helper function#246
InvincibleRMC wants to merge 2 commits intoros2:rollingfrom
InvincibleRMC:refactor-check-fields

Conversation

@InvincibleRMC
Copy link
Contributor

Description

For the work around making sequences cast to list it required several changes to the spaghetti which was the template around it. This PR only moves the spaghetti out into python where making these changes is easier to test going forward and easier to enforce coding standards. A big of the reason this was done is the upcoming draft of the PR I had it basically required running this block a second time in some cases and the way it was templated it just was not possible without just copying the whole block and hoping.

I tested this make no difference by running diff -r on the messages generated by base rolling and this branch to ensure there were no difference.

Is this user-facing behavior change?

No It should only be an internal design detail.

Did you use Generative AI?

Additional Information

Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
Signed-off-by: Michael Carlstrom <rmc@carlstrom.com>
@InvincibleRMC
Copy link
Contributor Author

@christophebedard would you be able to review this?

@christophebedard
Copy link
Member

I'm not a big fan of doing (basically) template expansion in Python files directly; I think in general we should be moving the other way, to .em files. Is there another way to keep the code in an .em file and still be able to re-use it? Maybe have a separate .em file that we can include multiple times? I'm not very familiar with this

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.

2 participants