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.

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

Remove unused attributes from robot.running.Keyword model object #4589

Closed
pekkaklarck opened this issue Jan 9, 2023 · 0 comments
Closed

Comments

@pekkaklarck
Copy link
Member

pekkaklarck commented Jan 9, 2023

We have Keyword model objects both under robot.running and robot.result and they also have a common base class under robot.model. The running side Keyword represent keyword calls like

Log    Hello, world!
${x} =    Keyword

that consists of only keyword name, argument and possible assignment. At this point the keyword to actually run hasn't been determined so there's no knowledge about its documentation, possible child keywords, and other such info. On the result side Keyword represents an executed keyword and has much more information like documentation, execution times and child keywords.

At the moment our robot.running.Keyword has doc, tags, timeout and teardown attributes that aren't actually used for anything. I noticed them when prototyping to_json functionality as part of #3902. It makes no sense to serialize these unused attributes to JSON, or to load them from JSON, and I believe it's best to remove them altogether.

These attributes should also be removed from the robot.model.Keyword base class and moved fully to robot.running.Keyword. Documentation needs to also be enhanced to make it explicit that on the running side Keyword represents a keyword call and on the result side it represents an executed keyword. In a retrospect the running side object perhaps should be called KeywordCall, but changing that would be too badly backwards incompatible change (it would break all visitors) that it makes no sense.

Also removing these attributes is a backwards incompatible change because someone may have programmatically created running side Keyword objects with them or someone may inspect them using visitors or otherwise. Backwards incompatible changes aren't great in a non-major release, but because these attributes aren't used for anything, it's unlikely users actually set or access them either. Such a usage could be considered a bug and should be fixed anyway. If we do the change early in RF 6.1 development cycle, users have a possibility to test preview releases and we can revert the change if needed. In that case these attributes would nevertheless be removed in RF 7.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant