-
Notifications
You must be signed in to change notification settings - Fork 3
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
added build from git option #14
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some small changes needed, as git ref is optional.
Besides that, maybe rebase from current main? There's no merge conflict, but as the rebase included a bunch of commits, a rebase (which should be trivial without conflict) would make for a slightly cleaner history.
As a side note: when testing this I noticed that the current version of the controller / pipeline for the gitRepo build type is not working - but that will be a separate story.
frontend/src/pages/CREResources/AddImagePage/AddNewImageForm.tsx
Outdated
Show resolved
Hide resolved
frontend/src/pages/CREResources/AddImagePage/AddNewImageForm.tsx
Outdated
Show resolved
Hide resolved
/deploy |
I tested the latest version and it The dashboard looks good, and it interacts with CREs correctly. I still have not managed to make it work end to end due to thoth-station/meteor-operator#162 Maybe by the time CI (prow) is back to handle tests and merging, e2e will be ready :) |
Unfortunately I have to correct: now that I did manage to test the PR end to end, I found that when the pipeline succeeds and the CRE is flagged as Thinking that it was something still missing / wrong in the gitRepo build type from the CRE/pipeline, I tested this with an imageImport build type and I saw the same behaviour: there seems to be something here that breaks correct status reporting at the end of the process - I couldn't spot it at first sight though |
Gage helped clarify that this comes from an inconsistency in the controller: thoth-station/meteor-operator#169 From the dashboard point of view, this works as expected. /lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: codificat 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 |
/retest |
Related Issues and Dependencies
fixes: #11
This introduces a breaking change
This should yield a new module release
This Pull Request implements
Creates the import from git repo option
Description