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.

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

fix(api): workflow as code improvements #3423

Merged
merged 9 commits into from
Oct 10, 2018
Merged

fix(api): workflow as code improvements #3423

merged 9 commits into from
Oct 10, 2018

Conversation

fsamin
Copy link
Member

@fsamin fsamin commented Oct 9, 2018

  1. Description
  • Since a hook is mandatory for workflow as code, do not export it
  • Do not export default VCS payload
  1. Related issues
  1. About tests
  2. Mentions

@ovh/cds

@ovh-cds

This comment has been minimized.

Sign in to view
@ovh-cds

This comment has been minimized.

Sign in to view
fsamin added 8 commits October 10, 2018 10:30
@fsamin
wip fix #3239
9a0c04d
@fsamin
wip
566dd2c
@fsamin
wip
5c8e119
@fsamin
fix #3239
d3a18dd
@fsamin
fix(api): workflow as code improvements
1f680e1 
* Since a hook is mandatory for workflow as code, do not export it
* Do not export default VCS payload

closes #3239
@fsamin
fix
60bc5dc
@fsamin
fix
9ed4562
@fsamin
fix
6466076
@fsamin fsamin force-pushed the fsamin/fix-2018-10-08-10-41-57 branch from a1e9010 to 6466076 Compare October 10, 2018 08:30
richardlt
richardlt requested changes Oct 10, 2018
View reviewed changes
sdk/cdsclient/client_export.go Outdated
path := fmt.Sprintf("/project/%s/pull/workflows/%s", projectKey, name)
if exportWithPermissions {
/*if exportWithPermissions {
Copy link
Member

Choose a reason for hiding this comment

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

comment

sdk/cdsclient/client_export.go Outdated
}
bodyReader, _, _, err := c.Stream(context.Background(), "GET", path, nil, true)
func (c *client) WorkflowExport(projectKey, name string, mods ...RequestModifier) ([]byte, error) {
/*
Copy link
Member

Choose a reason for hiding this comment

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

comment

sdk/workflow.go Outdated
return has
}

if hasKey("git.branch") &&
Copy link
Member

Choose a reason for hiding this comment

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

return

sdk/workflow.go
dumper.ExtraFields.Len = false
dumper.ExtraFields.Type = false

mI, _ := dumper.ToMap(i)
Copy link
Member

Choose a reason for hiding this comment

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

Comment: // compare interface keys with default payload keys

sdk/workflow.go Outdated
mI, _ := dumper.ToMap(i)
mD, _ := dumper.ToMap(WorkflowNodeContextDefaultPayloadVCS{})
var kI, kD []string
for k := range mI {
Copy link
Member

Choose a reason for hiding this comment

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

if len(mI) != len(mD) {
return false
}
for k := range mD {
if ! hasKey(k) {
return false
}
}

better than reflect.DeepEqual ?

@fsamin fsamin merged commit 4d80334 into master Oct 10, 2018
@fsamin fsamin deleted the fsamin/fix-2018-10-08-10-41-57 branch October 10, 2018 17:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bnjjj bnjjj bnjjj approved these changes

@richardlt richardlt richardlt approved these changes

@yesnault yesnault yesnault approved these changes

@sguiheux sguiheux Awaiting requested review from sguiheux sguiheux is a code owner

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

Successfully merging this pull request may close these issues.

ascode: yml with hook
5 participants
@fsamin @ovh-cds @yesnault @richardlt @bnjjj