Skip to content

Conversation

@guicho271828
Copy link
Contributor

No description provided.

@mergify
Copy link

mergify bot commented Nov 5, 2025

Merge Protections

Your pull request matches the following merge protections and will not be merged until they are valid.

🟢 Enforce conventional commit

Wonderful, this rule succeeded.

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|docs|style|refactor|perf|test|build|ci|chore|revert|release)(?:\(.+\))?:

@guicho271828 guicho271828 force-pushed the powerups branch 3 times, most recently from 98703d1 to 83b699a Compare November 5, 2025 20:26
Copy link
Contributor

@jakelorocco jakelorocco left a comment

Choose a reason for hiding this comment

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

Not sure if these are just issues with my type-hinter / the default one in VSCode.

It may not be possible to dynamically create these methods while retaining basic intellisense and type hinting.

@guicho271828 guicho271828 marked this pull request as ready for review November 10, 2025 17:59
@guicho271828 guicho271828 force-pushed the powerups branch 3 times, most recently from d5e5efd to 3155c9f Compare November 10, 2025 18:44
Copy link
Contributor

@jakelorocco jakelorocco left a comment

Choose a reason for hiding this comment

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

this works for me; but doesn't offer any intellisense / tab completes / type hinting.

I don't think there's an extendable solution that updates the type completion. The best we could do is force modules to export a single protocol; something like:

class ThisSpecificPowerup(Protocol):
  def hello():
      ...

class PoweredUp(ThisSpecificPowerup, MelleaSession):
    ...

def apply(m: MelleaSession) -> PowerdUp
    ... # do work here to apply
    return m

m.hello() # this is properly type hinted / etc...

But this only works for one set of power ups at a time.

@guicho271828
Copy link
Contributor Author

Well, now this PR does not remove any feature (like the one with register commits which made docstrings invisible), only adds one which has a room of improvement for future revisions. Progress over Perfection, this should not preclude merging. I need this feature for internal va users.

Copy link
Contributor

@jakelorocco jakelorocco left a comment

Choose a reason for hiding this comment

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

allows the methods to be called; I will give up on type-hinting / intellisense

@guicho271828 guicho271828 merged commit 662cfcc into generative-computing:main Nov 12, 2025
1 of 4 checks passed
@guicho271828 guicho271828 deleted the powerups branch November 12, 2025 19:18
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