You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@devlake.apache.org by GitBox <gi...@apache.org> on 2022/09/26 06:50:47 UTC
[GitHub] [incubator-devlake] mintsweet opened a new pull request, #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
mintsweet opened a new pull request, #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191
…DORA tasks
### ⚠️ Pre Checklist
> Please complete _ALL_ items in this checklist, and remove before submitting
- [ ] I have read through the [Contributing Documentation](https://devlake.apache.org/community/).
- [ ] I have added relevant tests.
- [ ] I have added relevant documentation.
- [ ] I will add labels to the PR, such as `pr-type/bug-fix`, `pr-type/feature-development`, etc.
# Summary
<!--
Thanks for submitting a pull request!
We appreciate you spending the time to work on these changes.
Please fill out as many sections below as possible.
-->
**deploymentPattern** and **productionPattern** are not passed when DORA is not required.
### Does this close any open issues?
Closes #3189
### Screenshots
**DORA is required**
**DORA is not required**
### Other Information
Any other information that is important to this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] abeizn merged pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
abeizn merged PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] mintsweet commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
mintsweet commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980754320
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
I think there is some truth to what you said. Whether a controlled component can receive undefined depends on the definition of the component. The reason I avoid these is that the current component hierarchy is too deep. If you can reduce the calling level, use what you said. The way I think it would indeed be better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r979993879
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -96,6 +97,13 @@ const DataTransformations = (props) => {
cardStyle = {}
} = props
+ // Used to determine whether to display edit transformation or add transformation
+ const [initializeTransformations, setInitializeTransformations] = useState({})
Review Comment:
This is not an ideal naming choice as we `InitializeTransformations` is already in use at the DSM Hook level for representing the global Transformations map. Additionally, this extra check is not necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] mintsweet commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
mintsweet commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980653343
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -96,6 +97,13 @@ const DataTransformations = (props) => {
cardStyle = {}
} = props
+ // Used to determine whether to display edit transformation or add transformation
+ const [initializeTransformations, setInitializeTransformations] = useState({})
+
+ useEffect(() => {
Review Comment:
This is the same as the above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] mintsweet commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
mintsweet commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980653191
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -138,15 +146,20 @@ const DataTransformations = (props) => {
)
const [activeEntity, setActiveEntity] = useState()
- const transformationHasProperties = useCallback(
+ const transformationHasChanged = useCallback(
Review Comment:
Because "deploymentPattern" and "productionPattern" are empty, DORA-related tasks will be started, and DORA tasks will not be started when they do not exist. The original judgment here cannot support the situation that these two parameters are empty strings.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r979995015
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -96,6 +97,13 @@ const DataTransformations = (props) => {
cardStyle = {}
} = props
+ // Used to determine whether to display edit transformation or add transformation
+ const [initializeTransformations, setInitializeTransformations] = useState({})
+
+ useEffect(() => {
Review Comment:
This effect should be removed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980676456
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -96,6 +97,13 @@ const DataTransformations = (props) => {
cardStyle = {}
} = props
+ // Used to determine whether to display edit transformation or add transformation
+ const [initializeTransformations, setInitializeTransformations] = useState({})
Review Comment:
While I'm a fan of `lodash` and it has many useful helpers for working with arrays and objects etc., It's not really needed to get the existing logic working, and introduces another 3rd party dependency.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980674646
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
This is not the ideal solution using `undefined` values, I don't see the extra prop as being more maintenance. When using React controlled components, `undefined` cannot be assigned to input component values. This is introducing errors into the React console that will affect the production build. If empty string is not supported we'll need to add some post-processing to the board or project's transformation to exclude those properties from payload if empty. It would be easier for backend to treat empty strings as "disabled" and not execute DORA tasks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r982417915
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
@mintsweet Linting was also not run prior to finalizing this PR, and a missing Effect dependency has caused new warning.
<img width="800" alt="Screen Shot 2022-09-28 at 9 35 17 AM" src="https://user-images.githubusercontent.com/1742233/192792968-ff073739-098a-4817-b462-1624698659fb.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r982417915
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
Linting was also not run prior to finalizing this PR, and a missing Effect dependency has caused new warning.
<img width="800" alt="Screen Shot 2022-09-28 at 9 35 17 AM" src="https://user-images.githubusercontent.com/1742233/192792968-ff073739-098a-4817-b462-1624698659fb.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980059921
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
Using `undefined` for String inputs is not desired practice. When you set this to `undefined` it will propagate to `$activeTransformation` which is the main `$transformation` object at this layer, and React does not expect or want an undefined value for its input components.
It appears you are wanting to use `undefined` as a means to determine the "Off" state for radio selectors, when we should be using `empty` string instead (as originally designed).
If empty string means something else to the backend, we may need to consider using another string value to "disable" a deploy tag, perhaps "[disabled]" or "/disabled/"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r979994386
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -138,15 +146,20 @@ const DataTransformations = (props) => {
)
const [activeEntity, setActiveEntity] = useState()
- const transformationHasProperties = useCallback(
+ const transformationHasChanged = useCallback(
Review Comment:
This transformation check handler was already working properly, no refactoring is needed here please revert this entire file.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
Posted by GitBox <gi...@apache.org>.
e2corporation commented on PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#issuecomment-1257991509
@mintsweet Please remove the ":warning: Pre Checklist" Section and replace with an appropriate description if this PR is ready, otherwise tag it as a **Draft**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] mintsweet commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
mintsweet commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980652573
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -96,6 +97,13 @@ const DataTransformations = (props) => {
cardStyle = {}
} = props
+ // Used to determine whether to display edit transformation or add transformation
+ const [initializeTransformations, setInitializeTransformations] = useState({})
Review Comment:
This is to solve the button text display "add transformation" or "edit transformation" exists.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980125389
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
Alternatively, we could use `onSettingsChange` to create an unofficial prop in the transform object that is not directly bound to an input component, for the configured entity, called `useDeployment' that is a boolean or something, that you could even set to `undefined`. This way we can check `transformation?.useDeployment` to dictate the _selected_ state of the radio element.
If this field is useful to the backend as well, we could make it a formal property in the default transformations init setup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r982329081
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
@mintsweet Nesting level is not related to the fact that Controlled Input Components must not have undefined values during its lifecycle. I am going to refactor this all later to meet those original principles. This PR has caused 2 problems:
- Creating `[object]` and `[undefined]` Entries in main **Transformations** Map due to use of entityIdKey within an Effect instead of the original handler I had implemented.
- Improper use of controlled input switching from `undefined` to a controlled value during its lifecycle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r982329081
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
@mintsweet Nesting level is not related to the fact that Controlled Input Components must not be defined. I am going to refactor this all later to meet those original principles. This PR has caused 2 problems:
- Creating `[object]` and `[undefined]` Entries in main **Transformations** Map due to use of entityIdKey within an Effect instead of the original handler I had implemented.
- Improper use of controlled input switching from `undefined` to a controlled value during its lifecycle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r979993879
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -96,6 +97,13 @@ const DataTransformations = (props) => {
cardStyle = {}
} = props
+ // Used to determine whether to display edit transformation or add transformation
+ const [initializeTransformations, setInitializeTransformations] = useState({})
Review Comment:
This is not an ideal naming choice as the naming convention `InitializeTransformations` is already in use at the DSM Hook level for representing the global Transformations map. Additionally, this extra check is not necessary.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute …
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980125389
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
Alternatively, we could use `onSettingsChange` to create an unofficial prop in the transform object that is not directly bound to an input component, for the configured entity, called `useDeployment` that is a boolean or something, that you could even set to `undefined`. This way we can check `transformation?.useDeployment` to dictate the _selected_ state of the radio element.
If this field is useful to the backend as well, we could make it a formal property in the default transformations init setup.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980675730
##########
config-ui/src/components/blueprints/create-workflow/DataTransformations.jsx:
##########
@@ -96,6 +97,13 @@ const DataTransformations = (props) => {
cardStyle = {}
} = props
+ // Used to determine whether to display edit transformation or add transformation
+ const [initializeTransformations, setInitializeTransformations] = useState({})
Review Comment:
That behavior is working already working as desired, Edit transformation only shows if one of the props has a non-empty value. Your method of how deployment tags are being utilized is causing you to have to modify this logic.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] mintsweet commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
mintsweet commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r980655311
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
**deploymentPattern** and **productionPattern** are empty strings by default, DORA-related tasks will be executed at this time, and default by the backend will convert their values. when the request does not have these two parameters, DORA-related tasks Tasks will not be executed, set to undefined in the request parameters, and they will not be brought into the request parameters.
If using an extra parameter to manage them, I think this would incur additional maintenance costs.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r984094602
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
@mintsweet @likyh This effect will definitely need to be removed, adding the dependency here creates a circular loop resulting in a maximum nesting level error. This is why it's always extremely important to run lint locally and fix the react-hooks/exhaustive-deps warnings. I wanted to fix this on a new PR but I have to leave it unresolved as it's causing breakages.
<img width="1355" alt="Screen Shot 2022-09-29 at 7 26 02 PM" src="https://user-images.githubusercontent.com/1742233/193159592-bc0bdc6a-6883-433e-a628-214dcd30365c.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-devlake] e2corporation commented on a diff in pull request #3191: fix(config-ui): adjust the parameters so the devlake can not execute DORA tasks
Posted by GitBox <gi...@apache.org>.
e2corporation commented on code in PR #3191:
URL: https://github.com/apache/incubator-devlake/pull/3191#discussion_r984094602
##########
config-ui/src/components/blueprints/transformations/CICD/Deployment.jsx:
##########
@@ -40,17 +40,25 @@ const Deployment = (props) => {
useEffect(() => {
setSelectValue(
transformation?.deploymentPattern ||
- transformation?.deploymentPattern === ''
+ transformation?.deploymentPattern === '' ||
+ transformation?.productionPattern ||
+ transformation?.productionPattern === ''
? 1
: 0
)
- }, [transformation?.deploymentPattern])
+ }, [transformation?.deploymentPattern, transformation?.productionPattern])
const handleChangeSelectValue = (sv) => {
if (entityIdKey && sv === 0) {
- onSettingsChange({ deploymentPattern: undefined }, entityIdKey)
+ onSettingsChange(
+ { deploymentPattern: undefined, productionPattern: undefined },
Review Comment:
@mintsweet @likyh This effect will definitely need to be removed, adding the dependency here creates a circular loop resulting in a maximum nesting level error. This is why it's always extremely important to run lint locally and fix the react-hooks/exhaustive-deps warnings.
<img width="1355" alt="Screen Shot 2022-09-29 at 7 26 02 PM" src="https://user-images.githubusercontent.com/1742233/193159592-bc0bdc6a-6883-433e-a628-214dcd30365c.png">
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@devlake.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org