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

fix: Updated hackathon readme landing page path #893

Merged
merged 3 commits into from
Nov 5, 2021

Conversation

jeremytchang
Copy link
Collaborator

Updated to match what is running in production

Updated to match what is running in production
Copy link
Contributor

@jkaster jkaster left a comment

Choose a reason for hiding this comment

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

A suggestion and a question

@@ -60,7 +81,7 @@ application: hackathon_app {
Remember to add a model to the project that has a valid connection.

## Specific steps to `yarn`
1. run `yarn install` in sdk-codegen
1. run `yarn install` in sdk-codegen
2. run `yarn build` in sdk-codgen
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
2. run `yarn build` in sdk-codgen
2. run `yarn build` in sdk-codegen

@@ -50,7 +50,7 @@ const HACKATHON_USER_ATTR_NAME = 'hackathon' // case sensitive
const HACKATHON_USER_ATTR_LABEL = 'Hackathon'
const HACKATHON_GROUP_PREFIX = 'Looker_Hack: '
const LANDING_PAGE_ATTR_NAME = 'landing_page'
const LANDING_PAGE_PATH = '/extensions/hackathon::hackathon_app'
const LANDING_PAGE_PATH = '/extensions/hackathon::hackathon'
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the user registration script setting this path for the default landing page? Are all existing registrants going to have their homepage updated to this new URL?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The user registration group creates a group for hackathon 2021. The script assigns a role to the group that defines the landing page. When users are added, they are added to the group and thus get the landing page attribute overwritten by the group role.
So technically, this change shouldn't be needed. And i can jsut change our manifest.kml to rename to hackathon_app. But i want to be safe.

jkaster
jkaster previously approved these changes Nov 5, 2021
Copy link
Contributor

@jkaster jkaster left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@jkaster jkaster left a comment

Choose a reason for hiding this comment

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

LGTM

@jeremytchang jeremytchang merged commit 382a9f0 into main Nov 5, 2021
@jeremytchang jeremytchang deleted the jc/cleanup_hackathon branch November 5, 2021 23:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants