Open hhkaos opened 3 years ago
Let me share the notes from the meeting @kellyhutchins, @RalucaNicola and @ak-kemp just add.
From what I understood, we have agreed on the following:
Q0) Do we use \t instead of spaces? We agreed on using \t
Q1) The snippet should contain a new class initialization, its constructor properties, or both? We agreed on both are OK for JavaScript. For TypeScript only the "new ... " version. We agreed on adding the suffix "Props" to the version just with the properties (without "new").
Q2) When creating new class, do we also assign it to a variable? We agreed on not assigning it to a variable
Q3) Should we avoid adding multiple expressions in one snippet?. We agreed on trying to keep snippets small and try not to instantiate more than one class whenever possible.
Q4) Do we allow comments? We agreed on not using comments. Use the description instead.
Q5) When instantiating a new class, do we add only required fields, the most common ones, all? We think there is no unique answer for this. Try to use the most common ones but it will need to be checked case by case. We agreed on sorting properties by alphabetical order.
Please correct me if I understood something wrong.
Cheers
Q6: I prefer the snippet name too we'll just need to ensure that we add info to the readme to let users know how these snippets within snippets work. Q7: For simplicity sake I think we should leave out the placeholder that shows the options. I worry that it will be too hard to maintain and as you noted if users are working with the jsapi types they already get that info.
Q8: Interesting approach. I like it and think it would be a nice option that makes it generic.
Q9: I think it would be useful to have snippets for the enums and like the idea of adding the enum suffix.
I agree with Kelly on the last 4 questions, nothing to add.
Yuhu!, so we got to an agreement then. I will add the agreements to https://github.com/Esri/arcgis-js-vscode-snippets/blob/master/contributing.md :)
@kellyhutchins I think we should also add to the guidelines that after a colon, there needs to be a single space " ".
I'm not sure about singles quotes vs double quotes vs backticks 😅, do we follow any rule/convention?.
@hhkaos I think we can follow the same guidelines used here in the JSAPI resources: https://github.com/Esri/jsapi-resources/blob/main/.prettierrc.json
I think we can follow the same guidelines used here in the JSAPI resources: Esri/jsapi-resources@main/.prettierrc.json
Perfect!, we will have to include that in the contributing guidelines.
Another question related to the body conventions.
Properties for autocasting vs constructors
I have noticed most of the snippets today contain properties for autocasting:
{
type: "polygon-3d",
symbolLayers: [{
type: "fill",
pattern: {
type: "style",
style: "${1|solid,vertical,horizontal,forward-diagonal,diagonal-cross,cross,backward-diagonal|}"
},
material: { color: ${2:[255, 250, 239, 0.8]} },
outline: { color: ${3:[70, 70, 70, 0.7]}}
}]
}
But it could also be:
new PolygonSymbol3D({
symbolLayers: [{
type: "fill",
pattern: {
type: "style",
style: "${1|solid,vertical,horizontal,forward-diagonal,diagonal-cross,cross,backward-diagonal|}"
},
material: { color: ${2:[255, 250, 239, 0.8]} },
outline: { color: ${3:[70, 70, 70, 0.7]}}
}]
})
Almost the same except for the type: "polygon-3d",
many times needed for autocasting.
Should we remain consistent and recommend one approach versus the other? Or any rule to when to go for one vs the other? I just feel it can get messy and end-up having random patterns.
Raul
P.S. if we are OK with both, I wonder if we should apply different conventions for each (e.g. common properties vs all properties).
Hello again!,
Sorry I know this issue is long, but I wanted to share with you some doubts that have arisen while I was creating new snippets and my opinion about them.
As mentioned in the previous issue, I think it would be good if we could try to reach an agreement to define minimum conventions to facilitate the contributions and make them as heterogeneous as possible.
Q0) Do we use \t instead of spaces?
or
A: Yes, I think we should if we want to be more friendly to different tab configurations 😄
Q1) Then snippet should contain a new class initialization, its constructor properties, or both?
Ex:
simpleMarkerSymbol
snippet:or
A: In my opinion we could add both (and call them something like
simpleMarkerSymbol
,simpleMarkerSymbolProps
) or simply leave the second one. But in that case, I would add the Class name and the module path in the description.Q2) When creating a new class, do we also assign it to a variable?
Ex:
featureLayer
snippet:or
A: I think I would remove the variable. But I think it is a minor thing.
Q3) Should we avoid adding multiple expressions in one snippet?.
Ex:
webmap
snippet:or
featureLayer
snippet:or
disableNavigation
snippet:A: I think snippets should be as short as possible. If it is to help init a layer we should only do the init and nothing else. In some cases snippets like disable mapview navigation will be larger.
Q4) Do we allow comments?
Ex:
queryLayer
snippet:A: I think it is OK to add comments
Q5) When instantiating a new class, do we add only required fields, the most common ones, all?
E.x:
popupTemplate
snippet:A: I don't have a clear answer to this. I would probably add the most used ones with a
// recommended
comment or similar.Q6) When the property expects another class but supporting autocasting, what do we do?. Add a sample or place the snippet name?
E.x:
heatmapRenderer
snippet:or
A: I prefer the snippet name
Q7) When a property can hold different value types, should we add a placeholder to show all possible inputs?
E.x. PopupTemplate properties:
Do we do something like this?
A: I think it is nice to provide all possible input types, knowing that it might not always be up-to-date. Knowing that in case of been using @types/arcgis-js-api it might be kind of duplcated (but it still can add some value like specify the value that needs to be returned by the function, or the sintax to use in the
string
can referer to geometry fields:Q8) How do we create snippets for inherited methods
Ex: method fromJSON() inherited in the SimpleRenderer, UniqueValueRenderer, HeatmapRenderer, etc.
Do we do something like this?:
A: to make them generic I would try to do so.
Q9) Do we allow snippets for enums?
Ex:
A: I think I would allow them with the suffix "enum". Something like
rendererTypesEnum
,basemapsEnum
,symbolTypesEnum
, ...