Skip to content

Update default WorkOrder model #88

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Update default WorkOrder model #88

wants to merge 3 commits into from

Conversation

Yadunund
Copy link
Member

This PR

  • Gets rid of Item as a required field in the work order. This is replaced by MetaData which is optional in the work order. Users can directly access the yaml contents.
  • Gets rid of name inside Step as it was redundant.
  • Adds a getter for process_parameters in Step to return a YAML node of parameters. Downstream skills should have access to these parameters via Task.data in the context during configuration

@Yadunund Yadunund requested a review from aaronchongth April 17, 2025 22:23
aaronchongth
aaronchongth approved these changes Apr 18, 2025
View reviewed changes
Copy link
Collaborator

@aaronchongth aaronchongth left a comment

Choose a reason for hiding this comment

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

Looks good! Just left a question for consideration, but I'll leave it up to you since you have better context on the use-case or non-use-case.

nexus_common/include/nexus_common/models/work_order.hpp Outdated
#include "../yaml_helpers.hpp"

namespace nexus::common {

class WorkOrder
{
public: struct Item
public: struct MetaData
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is a pretty useful generic model, should we have it as standalone, instead of within the namespace of WorkOrder?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aaronchongth aaronchongth aaronchongth approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Yadunund @aaronchongth