fix: respect the reason required org setting for ignore creation#409
fix: respect the reason required org setting for ignore creation#409CatalinSnyk wants to merge 1 commit intomainfrom
Conversation
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) ✅ license/snyk check is complete. No issues have been found. (View Details) ✅ code/snyk check is complete. No issues have been found. (View Details) |
CatalinSnyk
force-pushed
the
fix/CLI-1087_iaw_reason_required_org_setting
branch
3 times, most recently
from
ed8b2ca to
cb1ddfc
Compare
CatalinSnyk
force-pushed
the
fix/CLI-1087_iaw_reason_required_org_setting
branch
from
cb1ddfc to
95b41b2
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
| }) | ||
| } | ||
|
|
||
| func getOrgIgnoreSettings(engine workflow.Engine) (*contract.OrgSettingsResponse, error) { |
There was a problem hiding this comment.
suggestion: pass in the necessary parameters only, instead of the entire engine
| return nil, err | ||
| } | ||
|
|
||
| engine.GetConfiguration().Set(ConfigIgnoreSettings, settings) |
There was a problem hiding this comment.
suggestion: we already have the config declared, any reason not to use it here?
| engine.GetConfiguration().Set(ConfigIgnoreSettings, settings) | |
| config.Set(ConfigIgnoreSettings, settings) |
| } | ||
| } | ||
|
|
||
| func getOrgIgnoreReasonRequired(engine workflow.Engine) configuration.DefaultValueFunction { |
There was a problem hiding this comment.
suggest: getOrgIgnoreReasonRequired, getOrgIgnoreSettingsConfig, getOrgIgnoreApprovalEnabled are all making use of getOrgIgnoreSettings. Is getOrgIgnoreSettings something that can be added to config via config.AddDefaultValue? That way we can take advangate of caching to save mulutiple API calls.
| return err | ||
| } | ||
|
|
||
| engine.GetConfiguration().AddDefaultValue(ConfigIgnoreSettings, getOrgIgnoreSettingsConfig(engine)) |
There was a problem hiding this comment.
suggest: looks like we do here, can we then not do a config.get(ConfigIgnoreSettings) instead of the repeated calls to getOrgIgnoreSettings?
CatalinSnyk
force-pushed
the
fix/CLI-1087_iaw_reason_required_org_setting
branch
from
95b41b2 to
883102c
Compare
j-luong
force-pushed
the
fix/CLI-1087_iaw_reason_required_org_setting
branch
from
883102c to
95b41b2
Compare
2 participants
Adds support for the
ReasonRequiredorg setting in regards to ignores. The changes add a new config value for it and adapt the validator function in regards to the setting.