Remove trailing slash from path variable values #648
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Paths in some workflows and tasks are configured via variables. Previously, when a value specified a folder path, a
trailing slash was added. The intent behind this was to make it slightly more clear that the value was referring to a
folder (e.g., `./` vs. `.`). However, this practice can be harmful in the case where the value is used as a base
component in a path, as then it is most appropriate to omit the path separator in the concatenation code, which is at
all clear (e.g., `FOO_PATH: foo/`, `{{.FOO_PATH}}bar` -> `foo/bar`). And if the separator is used in the concatenation
code, it results in a confusing double separator in the resulting path (e.g., `FOO_PATH: foo/`, `{{.FOO_PATH}}/bar` ->
`foo//bar`). The benefit of the trailing slash on the variable definition is miniscule at most, since the variable name,
documentation, and other context should make it obvious that the value is a folder path. So the harm of this approach
outweighs the benefit.
For this reason, it is better to omit the trailing slash in the variable definition (e.g., `FOO_PATH: foo`,
`{{.FOO_PATH}}/bar` -> `foo/bar`). Even though this approach is not relevant in cases where the path is not used as a
base component, it is best to be consistent in this practice.
per1234
added
type: enhancement
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Paths in some workflows and tasks are configured via variables. Previously, when a value specified a folder path, a trailing slash was added. The intent behind this was to make it slightly more clear that the value was referring to a folder (e.g.,
./vs..). However, this practice can be harmful in the case where the value is used as a base component in a path, as then it is most appropriate to omit the path separator in the concatenation code, which is at all clear (e.g.,FOO_PATH: foo/,{{.FOO_PATH}}bar->foo/bar). And if the separator is used in the concatenationcode, it results in a confusing double separator in the resulting path (e.g.,
FOO_PATH: foo/,{{.FOO_PATH}}/bar->foo//bar). The benefit of the trailing slash on the variable definition is miniscule at most, since the variable name, documentation, and other context should make it obvious that the value is a folder path. So the harm of this approach outweighs the benefit.For this reason, it is better to omit the trailing slash in the variable definition (e.g.,
FOO_PATH: foo,{{.FOO_PATH}}/bar->foo/bar). Even though this approach is not relevant in cases where the path is not used as a base component, it is best to be consistent in this practice.