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

chore: start to clean kubeinvaders.js #87

Closed
wants to merge 1 commit into from

Conversation

gdorsi
Copy link

@gdorsi gdorsi commented Oct 16, 2024

Part of #82


return prefix + suffix;
}

var codename = getCodeName();
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Co-located getCodeName near to where is used.

async function checkHTTP(url, elementId) {
const response = await fetch(url);
$("#" + elementId).val(response.status);
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified checkHTTP using fetch --> https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API

Object.keys(queryParams).forEach(key => url.searchParams.set(key, queryParams[key]));
}

return fetch(url);
}
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created this helper function to reduce the code needed to send GET requests to k8s_url

for (var i = 0;i < lines.length;i++){
metric = lines[i].split(' ');
async function getMetrics() {
const response = await httpGetToK8S("/metrics");
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched from XMLHttpRequest to httpGetToK8S

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also used async/await to not rely on callbacks and make the code seamless syncronous.

const text = await response.text();

for (const line of text.split('\n')) {
const [key, value] = line.split(' ');
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Switched from:

        var lines = this.responseText.split('\n');
        for (var i = 0;i < lines.length;i++){
            metric = lines[i].split(' ');

Also I'm using the deconstructing assignement to give a name to the key/value pair

@gdorsi gdorsi marked this pull request as ready for review October 17, 2024 08:38
@gdorsi
Copy link
Author

gdorsi commented Oct 17, 2024

Ah, someone else already completed the task!

Closing in favour of #88

@gdorsi gdorsi closed this Oct 17, 2024
@lucky-sideburn
Copy link
Owner

hi @gdorsi

ok thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants