-
Notifications
You must be signed in to change notification settings - Fork 189
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 live build status to Telescope dashboard: #2582
Conversation
Can you update the branch and add an instruction for testing this feature? |
@humphd Since there's no way to run the autodeployment locally, maybe it's easier to point this to the stagging or prod autodployment?
|
@Andrewnt219 great suggestion. For reviewing this, that works, and we can change it back before we merge. @mqnguyen5 can you point it at https://dev.telescope.cdot.systems for now? |
Will do |
resMsg.toggleAttribute('hidden'); | ||
buildHeaderInfo.toggleAttribute('hidden'); |
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.
This makes the build header switch between loading state and info state every 5s. Like, 5s of info and 5s of loading.
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.
You have any suggestions on how to fix it? I've tried using setAttribute
and removeAttribute
here but it doesn't seems to do the trick
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.
Maybe the function should only render the build info, not toggling between two states (i.e. only render info state)? I don't think there's a case where toggling between those two is necessary.
change inside the |
@mqnguyen5 I changed, but the result is still display loading status. Do I need to add a specific address? |
@Yoda-Canada sorry, I just updated the instruction to test this feature. You mind trying it again? |
@manekenpix Sorry about that, you mind showing me a better way to do it instead of |
Use |
@mqnguyen5 I followed your new steps, and it only displayed the loading page. Do you use a specific address to get the following data? |
@mqnguyen5 Handlebars PR is merged, can you rebase this? The changes in build.html go into builds.hbs |
Will do, I'll try to get it done soon. |
https://en.wikipedia.org/wiki/Exit_status In Unix, a zero exit code means it worked, anything else is an error. |
@mqnguyen5 your code |
Good to clarify, there are lots of moving pieces here. |
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.
LGTM. Can you rebase and revert the change to autodeploymentUrl
?
0aa1517
to
6202306
Compare
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.
Two small things, looks good otherwise.
Fix and rebase please.
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.
Not sure if it looks good this way
Maybe you can hide the part below on loading
You can add hidden
in element of id="build-header-info"
and have a quick check if elem build-header-info
is hidden or not after line 12 in src/api/status/public/js/build-log/build-header.js
if (buildHeaderInfo.hidden) {
buildHeaderInfo.removeAttribute('hidden');
}
6202306
to
08743d8
Compare
* added scripts for fetching and rendering build-header * added html template for build-header * imported build-header to run on load made adjustments based on feedbacks: * removed fetch from build-header * moved build-header call to check-for-build so they are in sync * added extra return data (githubData, stop date, result code) from API * changed auto-deployment url to https://dev.telescope.cdot.systems * moved the build-header function call to before the if statement * removed the state toggling code * moved build-headers into .hbs file * changed returning stoppedAt value to use current date * removed the line setting innerText for build-header title * modified build-header to use card component from material dashboard * centered build-header title * reverted autodeployment url * hid build-header info when in loading state and show when data is fetched * removed the use of setAttribute and replaced them with setter on the DOM element API
cfda0a5
to
9a1ab98
Compare
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.
Looks great. Well done 👍.
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.
Great!You've really done hard work.
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.
LGTM!
Issue This PR Addresses
Fixes #2416
Type of Change
Description
To test the feature locally, do the following:
autodeploymentUrl
inpublic/js/buildl-log/api.js
connectSrc
insrc/server.js
current
toprevious
insidesrc/api/status/public/js/build-log/api.js
Checklist