Add panelling to Import Tekton resources from repository #182
Conversation
tekton-robot
added
the
size/XL
There was a problem hiding this comment.
Required fields should use the required attribute for accessibility / general UX purposes.
When there are no service accounts available the drop down should probably indicate this either by being disabled or including a 'no service accounts' message.
I'm also not seeing any feedback if I try to submit an empty form.
The design / layout should follow the Carbon guidelines, e.g.:
See https://www.carbondesignsystem.com/components/form/code and https://www.carbondesignsystem.com/components/form/style for details of the design system's approach for forms.
tekton-robot
added
size/L
tekton-robot
added
approved
tekton-robot
added
size/L
|
@AlanGreene in terms of feedback on an invalid URL I am not sure what kind of checks we should be doing and where/how to go about them 🤔 the API does some basic checking at the moment for slashes and this is when and error is returned but it is not a full check for a valid URL |
|
We should display something useful to the user, e.g. 'unable to load repo' / 'repo not found' or if we have any useful info in the API response. Right now it seems like nothing is happening when clicking the button which is not good. |
|
http://example.com reports invalid input for me when I try to submit as should I think any URL that isn't of the form I don't think we can do any actual checking of the URL so whether the repo is found as we may not even have access to the repo without the relevant permissions, the PipelineRun should run if the correct format is submitted and it's logs will report if there was a problem with the repo or accessing it |
Discovered in #182 (comment) that our JS linter is not being run on PR builds. Add it to node_test so we can capture these issues and provide faster feedback to developers without requiring manual review/testing of code style, unused variables/imports etc. Also add the node-* folder (used in PR builds) to .gitignore to avoid issues with unexpected lint config references.
Discovered in #182 (comment) that our JS linter is not being run on PR builds. Add it to node_test so we can capture these issues and provide faster feedback to developers without requiring manual review/testing of code style, unused variables/imports etc. Also add the node-* folder (used in PR builds) to .gitignore to avoid issues with unexpected lint config references.
|
/retest |
* add navigation to view pipelines and tasks remove logging statement update based on feedback update based on feedback - add tooltip, fix text wrapping style, clean up map state to props, remove unused async fix runs link * secret management * feedback updates * more feedback changes * Create pipelinerun (#2) * add serviceaccounts dropdown component * add a pipelines dropdown component * working on create pipelinerun * reset selectedItem when selected namespace changes * fix typo * add a pipelines dropdown component * add serviceaccounts dropdown component * use TooltipDropdown * add button linking to create-pipelinerun page * fixed selectItem in ServiceAccounts dropdown * link to my create page * add validation components * add style * fix bug where pipelineRun.status.conditions is undefined * got validation working with modal using ComposedModal * add styling * update labels * debugging validation * add openshift/minishift install of dashboard * Initial publish script for dashboard image This script is called as a postsubmit and publish the dashboard image to gcr.io/tekton-nightly/github.com/… using `ko` Signed-off-by: Vincent Demeester <vdemeest@redhat.com> * Add 'npm run lint' to PR build Discovered in tektoncd#182 (comment) that our JS linter is not being run on PR builds. Add it to node_test so we can capture these issues and provide faster feedback to developers without requiring manual review/testing of code style, unused variables/imports etc. Also add the node-* folder (used in PR builds) to .gitignore to avoid issues with unexpected lint config references.
|
This is ready for another review when you have a chance to take a look please @AlanGreene 👼 I think the only part of your comments that I haven't addressed here is setting the dropdown to be deactivated or to display some message when there are no ServiceAccounts, I think this should be done in the component so I can do this work separately from this issue |
|
@AlanGreene @jessm12 I am addressing the empty dropdown message in PR #201, which can go in after Jess's PR (I expect there will be a few merge conflicts) |
Add Pipeline0 and Pipeline0 task yaml definitions that are applied at install (ko apply) time, this pipeline applies all resources under the specified directory at the repository provided. Add the panel and associated tests to allow the user to input a repository URL and directory path and ServiceAccount and trigger a PipelineRun of Pipeline0 from this data.
|
I think this is hopefully 🤞 ready to go now @AlanGreene 😅 |
|
In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AlanGreene, jessm12 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
tekton-robot
added
the
approved
Add an overlay for the dashboard in the dogfooding cluster. The overlay includes: - an ingress resource - setting the service to "nodePort", which is required for the ingress to work with GKE. This patch includes also a ClusterIssuer that can be used for any ingress in the cluster. Using the cluster issuer requires cert-manager to be installed first: https://cert-manager.io/docs/installation/kubernetes/#installing-with-regular-manifests GKE load balancer setup the health check to point to the readiness probe defined in the pods associated to the service. The probe is expected to return 200, while in the latest dashboard it returns 204 No Content. This patch fixes the issue: tektoncd#1017 Partially fixes tektoncd#182
Changes
Add Pipeline0 and Pipeline0 task yaml definitions that are applied at install (ko apply) time, this pipeline applies all resources under the Pipeline's specified directory at the provided repository. Add the panel and associated tests to allow the user to input a repository URL and ServiceAccount and trigger a PipelineRun of Pipeline0 from this data.
Below is a screenshot of the panel:
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
See the contribution guide
for more details.