-
Notifications
You must be signed in to change notification settings - Fork 25
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
feature: custom fields support #23
Conversation
…ainst 'custom_field'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two minor comments. As far as I can see, it looks good. However, I only can actually test when the version is released.
README.md
Outdated
@@ -22,6 +22,8 @@ Since the issues and pull requests from this repository are also managed by this | |||
| `project_id` | true | The projectboard id. | | |||
| `resource_node_id` | true | The id of the resource node. | | |||
| `status_value` | false | The status value to set. Must be one of the values defined in your project board **Status field settings**. If left unspecified, new items are added without an explicit status, and existing items are left alone. | | |||
| `operation_mode` | true | The operation mode to use. Must be one of `custom_field`, `status`. Defaults to: `status` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to use both? So that you can use in operation mode 'custom_field', also a status change. Example: when an issue is created, it will be added to project x in status, todo. but also with iteration x.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can define either custom_field or status mode, but not both at the same time. If you want to define both at once, you can simply define that change within the json (as single select). Otherwise, you can also define two separate steps:
- Change status
- Change of the custom field
action.yml
Outdated
@@ -46,18 +64,43 @@ runs: | |||
script: | | |||
core.setFailed('user and organization field is set. Only one is supported.') | |||
|
|||
- name: "Execute Organization automation" | |||
if: inputs.organization != '' | |||
- name: "Check if operation_mode is set and custom_field_values is not nil" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think a typo: nil = null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It depends on the language. For example, in Go there is no null, but instead you use nil. To clarify this for more users, I think you are right that this should be changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Haven't been able to test this out but from the face of the code, it looks good to me.
I wonder if it would be nice to fold status
, operation_mode
, and custom_field_values
into a single field ? Currently, using custom_field_values
one can set the status
- if we were to provide a default for type
(to single_select_field
maybe), it would make it using the custom field simpler (this would be a breaking change, hence maybe it's not a good path forward).
README.md
Outdated
@@ -22,6 +22,8 @@ Since the issues and pull requests from this repository are also managed by this | |||
| `project_id` | true | The projectboard id. | | |||
| `resource_node_id` | true | The id of the resource node. | | |||
| `status_value` | false | The status value to set. Must be one of the values defined in your project board **Status field settings**. If left unspecified, new items are added without an explicit status, and existing items are left alone. | | |||
| `operation_mode` | true | The operation mode to use. Must be one of `custom_field`, `status`. Defaults to: `status` | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this defaults to status
, should it be non-required ? Being not-required would also make the next release not a breaking one, as far as I'm aware.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My understanding of the action definition is that it should not be necessary to specify the value if the default option is pre-filled. The snippet from below defines this field.
operation_mode:
description: |
Specifies the operation mode.
The value must be of type string.
The value can be one of the following:
- custom_field
- status
required: true
default: "status"
Since this ** required ** setting is not required when a default value is specified, I think you are right that we do not need to make this field required.
Do you have a blocker that prevents this function from being merged? Otherwise, I would like to merge this feature over the weekend. |
For me not, I really would like the merge so that i can use the new functionality on monday! |
Same, looking forward to using this new functionality! |
Closes: #22
The following example workflow assumes that your project has following fields defined (non default):
Option 1
,Option 2
,Option 3
Iteration 1
,Iteration 2
,Iteration 3
For more checkout the README