jbampton commented on a change in pull request #1661:
URL: https://github.com/apache/apisix-dashboard/pull/1661#discussion_r610308972
##########
File path: web/src/components/Upstream/UpstreamForm.tsx
##########
@@ -688,6 +688,19 @@ const UpstreamForm: React.FC<Props> = forwardRef(
})}
</Select>
</Form.Item>
+ <Form.Item
+ label={formatMessage({ id: 'page.upstream.step.scheme' })}
+ name="scheme"
+ >
+ <Select disabled={readonly}>
+ {['http', 'https', 'grpc', 'grpcs'].map((item, index) => (
+ <Select.Option value={item} key={index}>
Review comment:
In computer science, an associative array, map, symbol table, or
dictionary is an abstract data type composed of a collection of (key, value)
pairs, such that each possible key appears at most once in the collection
https://en.wikipedia.org/wiki/Associative_array
So my view is that `key` should come before `value` and also based on order
alphabetically.
```suggestion
<Select.Option key={index} value={item}>
```
##########
File path: web/src/components/Upstream/UpstreamForm.tsx
##########
@@ -688,6 +688,19 @@ const UpstreamForm: React.FC<Props> = forwardRef(
})}
</Select>
</Form.Item>
+ <Form.Item
+ label={formatMessage({ id: 'page.upstream.step.scheme' })}
+ name="scheme"
+ >
+ <Select disabled={readonly}>
+ {['http', 'https', 'grpc', 'grpcs'].map((item, index) => (
Review comment:
I have never done `.tsx` before. I have done one other `.tsx` review.
So similar my view is that an array or list should be ordered somehow and
mostly its done alphabetically. I always like rules, standards and style
guides.
```suggestion
{['grpc', 'grpcs', 'http', 'https'].map((item, index) => (
```
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
