Skip to content
This repository has been archived by the owner on Feb 17, 2020. It is now read-only.

Cleanup #3

Merged
merged 3 commits into from Jan 24, 2018
Merged

Cleanup #3

merged 3 commits into from Jan 24, 2018

Conversation

rock3r
Copy link
Contributor

@rock3r rock3r commented Jan 23, 2018

Problem

There's some outstanding Lint warnings on the code; we also want to get rid of some sketchy variable names on the Android side to map to the Firestore track data, so those snake case fields should be renamed to camel case.

Solution

Address all ESLint warnings (mostly imports); rename the fields for TrackData that are in snake_case to camelCase (a corresponding PR is open on squanchy-android)

Test(s) added

No tests — yet!

Paired with

Nobody

Copy link
Contributor

@fourlastor fourlastor left a comment

Choose a reason for hiding this comment

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

Gotta check what's up with the local optional (I'm not sure we can simply drop in the other one)

Other than that good stuff, just one comment

@@ -36,9 +36,9 @@ export interface Place {
export interface Track {
id: string
name: string
accent_color: Optional<string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Reminder for tomorrow : this should be using the correct optional

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll leave this to you then 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tracked in #4

@@ -91,3 +91,16 @@ export const generateSchedule = (firebaseApp: FirebaseApp) => (_: Request, respo
response.status(200).send('Yay!')
})
}

const trackFrom = (rawTrack: TrackData & { id: string } | null): Track | null => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can use WithId

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@fourlastor fourlastor merged commit a4983a3 into master Jan 24, 2018
@fourlastor fourlastor deleted the cleanup branch January 24, 2018 00:01
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants