Skip to content

Allow the type of self.sim_cfg not to have commit/commit_short#148

Draft
rswarbrick wants to merge 2 commits intolowRISC:masterfrom
rswarbrick:deploy-needs-commit-field
Draft

Allow the type of self.sim_cfg not to have commit/commit_short#148
rswarbrick wants to merge 2 commits intolowRISC:masterfrom
rswarbrick:deploy-needs-commit-field

Conversation

@rswarbrick
Copy link
Copy Markdown
Contributor

NOTE: This is a draft because it depends on #146 (the first commit in the PR)

This wouldn't work at runtime, but postponing the check until then is a reasonable stop-gap that allows us to postpone working out which subclasses of FlowCfg define commit/commit_short until we have tidied up the types in more of those classes.

Note: we can't do this by checking if self.sim_cfg is of type SimCfg,
because that would give a circular dependency between job.deploy and
sim.flow.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
@rswarbrick rswarbrick marked this pull request as draft April 2, 2026 18:30
This wouldn't work at runtime, but postponing the check until then is
a reasonable stop-gap that allows us to postpone working out which
subclasses of FlowCfg define commit/commit_short until we have tidied
up the types in more of those classes.

Signed-off-by: Rupert Swarbrick <rswarbrick@lowrisc.org>
Copy link
Copy Markdown
Contributor

@AlexJones0 AlexJones0 left a comment

Choose a reason for hiding this comment

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

Maybe I missed something, but I think this is now redundant after #139 was merged earlier today, perhaps you need to pull in the latest master? That PR moved the commit/commit_short attributes to be defined on the base FlowCfg, so now I would expect it should always exist and the typing shouldn't complain.

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