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

refactor(auth): move startUrls to constants file #5998

Merged
merged 4 commits into from
Nov 13, 2024

Conversation

nkomonen-amazon
Copy link
Contributor

Problem

Static start urls are scattered

Solution

Move them to the auth constants file

Additional

Created a file for datetime related code. It includes static values which represent certain amounts of time in milliseconds.
Future work will use this.


License: I confirm that my contribution is made under the terms of the Apache 2.0 license.

@nkomonen-amazon nkomonen-amazon requested a review from a team as a code owner November 13, 2024 17:15
@@ -0,0 +1,10 @@
/*!
Copy link
Contributor

Choose a reason for hiding this comment

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

Future idea: Now that we have this module, maybe the datetime formatting functions should live in it

// Given number of milliseconds elapsed (ex. 4,500,000) return hr / min / sec it represents (ex. "1 hr 15 min")
export function convertToTimeString(durationInMs: number) {
const time = new Date(durationInMs)
const hours = time.getUTCHours()
const minutes = time.getUTCMinutes()
const seconds = time.getUTCSeconds()
let timeString = `${seconds} sec`
if (minutes > 0) {
timeString = `${minutes} min ${timeString}`
}
if (hours > 0) {
timeString = `${hours} hr ${timeString}`
}
return timeString
}
// Given Date object, return timestamp it represents (ex. "01/01/23, 12:00 AM")
export function convertDateToTimestamp(date: Date) {
return date.toLocaleDateString('en-US', {
month: '2-digit',
day: '2-digit',
year: '2-digit',
hour: '2-digit',
minute: '2-digit',
})
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good callout, yeah I agree anything datetime related should just live in this module. I've moved the time related functions from this file in a new commit.

Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
It now makes sense for some existing functions to be in this new module

Signed-off-by: nkomonen-amazon <nkomonen@amazon.com>
@nkomonen-amazon nkomonen-amazon requested a review from a team as a code owner November 13, 2024 20:18
@nkomonen-amazon nkomonen-amazon merged commit 9ca5f13 into aws:master Nov 13, 2024
23 of 25 checks passed
@nkomonen-amazon nkomonen-amazon deleted the authRefactor branch November 13, 2024 21:02
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