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

v08 spec alignment - Workflow name no longer a required property #81

Merged
merged 2 commits into from
Sep 29, 2022

Conversation

spolti
Copy link
Member

@spolti spolti commented Sep 27, 2022

Signed-off-by: spolti filippespolti@gmail.com
issue: #70

Many thanks for submitting your Pull Request ❤️!

What this PR does / why we need it:

Special notes for reviewers:

Additional information (if needed):

Signed-off-by: spolti <filippespolti@gmail.com>
@ricardozanini
Copy link
Member

@lsytj0413 wanna take a look?

model/workflow.go Outdated Show resolved Hide resolved
}, "./testdata/workflows/checkinbox.cron-test.sw.yaml": func(t *testing.T, w *model.Workflow) {
},
"./testdata/workflows/checkinbox.cron-test.sw.yaml": func(t *testing.T, w *model.Workflow) {
assert.Equal(t, "Check Inbox Workflow", w.Name)
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should organize the testcase later, with all field assert?

Copy link
Member Author

Choose a reason for hiding this comment

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

not sure what do you mean.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for did not make it clear. Should we change the testcode to:

"./testdata/workflows/checkinbox.cron-test.sw.yaml": func(t *testing.T, w *model.Workflow) {
			assert.Equal(t,  expectWorkflowObject, w)
}

, maybe this is more effective, it can verify all of workflow fields.

Signed-off-by: spolti <filippespolti@gmail.com>
@spolti
Copy link
Member Author

spolti commented Sep 29, 2022

I think it is ready to merge.

@lsytj0413
Copy link
Collaborator

/lgtm

@ricardozanini ricardozanini merged commit 503471e into serverlessworkflow:main Sep 29, 2022
@ricardozanini ricardozanini added this to the Spec 0.8 Support milestone Sep 29, 2022
@spolti spolti deleted the nameNotRequired branch September 29, 2022 14:50
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.

None yet

3 participants