Skip to content
This repository has been archived by the owner on Jul 26, 2024. It is now read-only.

fix: quick fixes to support the new adM API #78

Merged
merged 2 commits into from
May 12, 2021
Merged

fix: quick fixes to support the new adM API #78

merged 2 commits into from
May 12, 2021

Conversation

pjenvey
Copy link
Member

@pjenvey pjenvey commented May 10, 2021

Description

  • specify form-factor & os-family
  • fixup region-code & handling of GCP load balancer's
    "client_region_subdivision" format
  • hardcode placement to "newtab"

Testing

This is a quick fix -- to be tested on the dev env deployment

Issue(s)

Closes #77

@pjenvey pjenvey requested a review from a team May 10, 2021 23:38
- specify form-factor & os-family
- fixup region-code & handling of GCP load balancer's
  "client_region_subdivision" format
- hardcode placement to "newtab"

Closes #77
src/server/cache.rs Outdated Show resolved Hide resolved
src/web/user_agent.rs Outdated Show resolved Hide resolved
src/web/user_agent.rs Outdated Show resolved Hide resolved
// client_region_subdivision: a "Unicode CLDR subdivision ID,
// such as USCA or CAON"
if subdivision.len() < 3 {
subdivision
Copy link
Collaborator

Choose a reason for hiding this comment

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

I wonder if we'd ever hit this branch as according to the doc the first two letters should always be the ISO-3166-2 country code. Just be safe ;)

Copy link
Member

@jrconlin jrconlin May 12, 2021

Choose a reason for hiding this comment

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

mumble mumble never trust outside data to be consistent mumble mumble

Copy link
Collaborator

@ncloudioj ncloudioj left a comment

Choose a reason for hiding this comment

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

LGTM, r=nanj

Thanks!

@pjenvey pjenvey merged commit 8051879 into main May 12, 2021
@pjenvey pjenvey deleted the fix/77 branch May 12, 2021 22:39
@pjenvey pjenvey mentioned this pull request May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adjust to new adM API
3 participants