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

Locale Data in Enviornment Variable #71

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Locale Data in Enviornment Variable #71

wants to merge 1 commit into from

Conversation

CKegel
Copy link
Contributor

@CKegel CKegel commented Mar 15, 2023

Add an endpoint for geographic info and move it from whiteboard.js to the enviornment file

@CKegel CKegel requested review from ddbruce and lramos15 as code owners March 15, 2023 18:03
@CKegel CKegel changed the base branch from master to dev March 15, 2023 18:03
@CKegel CKegel mentioned this pull request Mar 15, 2023
Copy link
Member

@ddbruce ddbruce left a comment

Choose a reason for hiding this comment

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

As I wrote in one location, I think changing the JS variables (server + client sides) to lat and long would be good.

Comment on lines +229 to +235
const response = fetch('/locale')
.then((response) => response.json())
.then((data) => {
locale = data;
updateDate();
setInterval(() => updateDate(), 2000);
});
Copy link
Member

Choose a reason for hiding this comment

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

This can be handled without the let locale; up above, particularly because locale should be immutable. Maybe something like:

const locale = await fetch("/locale");
const [{lat}, {long}] = locale.json();
setInterval(() => updateDate(lat, long), 2000);

This may not be entirely correct syntax-wise, but I think it's a sleeker way to handle it than using .then(). I realize it involves slight restructuring of updateDate(), but it could be handled in other ways, too. You may be able to leave lat and long as globals consts and be done. Let me know your thoughts.

@@ -24,6 +24,7 @@ const pool = mariadb.createPool({
const PORT = process.env.PORT || 8080;
const WEBSITE_ACCESS_TOKEN = process.env.WEBSITE_ACCESS_TOKEN;
const HERALD_TOKEN = process.env.HERALD_TOKEN;
const LOCALE = {longitude: process.env.LONGITUDE, latitude: process.env.LATITUDE};
Copy link
Member

Choose a reason for hiding this comment

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

Please abbreviate the JS variables to lat and long, respectively, just for brevity's sake. No problem keeping the env vars as the full word—that's your choice.

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