Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document all locations that variable substitution is available #2605

Closed
ghost opened this issue May 12, 2020 · 8 comments · Fixed by #2927
Closed

Document all locations that variable substitution is available #2605

ghost opened this issue May 12, 2020 · 8 comments · Fixed by #2927
Assignees
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation.

Comments

@ghost
Copy link

ghost commented May 12, 2020

Expected Behavior

We should write down all the places that a variable can be substituted in our CRDs.

Actual Behavior

It's a bit hit-or-miss; we demonstrate some places to substitute in this section: https://github.com/tektoncd/pipeline/blob/master/docs/tasks.md#using-variable-substitution but it doesn't look like we document all of the possible locations that are supported.

Proposal

My suggestion would be to update docs/variables.md with a table of all locations in our CRDs where variables will be successfully interpolated into.

Here are a couple places in our code where we perform substitutions:

So documenting the locations would just require reading the various funcs called from these spots and recording all the fields that get modified. Not entirely sure if these are the only places that we apply replacements from though.

@ghost ghost added good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation. labels May 12, 2020
@psschwei
Copy link
Contributor

psschwei commented Jul 1, 2020

I'll look into this one this week.

/assign @psschwei

@bobcatfish
Copy link
Collaborator

Hey @psschwei It looks like we actually already have this https://github.com/tektoncd/pipeline/blob/master/docs/variables.md 😨

If there's anything missing tho plz add :D otherwise maybe we should close this issue? Sorry for the confusion 😨 😨 😨

@ghost
Copy link
Author

ghost commented Jul 6, 2020

Just to clarify - this issue is about documenting where the variables can be used. The existing variables.md contents only document what the available variables are.

@bobcatfish
Copy link
Collaborator

Thanks @sbwsg ! Looks like there is work to do here after all, sorry for the (increased) confusion @psschwei XD

@psschwei
Copy link
Contributor

psschwei commented Jul 6, 2020

No worries, thanks for the clarification!

@pritidesai
Copy link
Member

@psschwei feel free to create PR with subset of changes if it's easier for you. There are many places variable substitutions happen at Task level, Pipeline level, etc. As a newbie to source code, might be easier to propose small changes 🥰

@pritidesai
Copy link
Member

@sbwsg do we consider task results substitutions as a variable substitution ? 🤔 I would think so, right?

value: $(tasks.mytask.results.sum)

@ghost
Copy link
Author

ghost commented Jul 8, 2020

Yeah I would definitely consider that a variable substitution as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Denotes an issue ready for a new contributor, according to the "help wanted" guidelines. kind/documentation Categorizes issue or PR as related to documentation.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants