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

Update marketing site URL #1321

Merged
merged 1 commit into from
Apr 5, 2017
Merged

Update marketing site URL #1321

merged 1 commit into from
Apr 5, 2017

Conversation

zachmargolis
Copy link
Contributor

Why: We're making www. the canonical domain for marketing

**Why**: We're making www. the canonical domain for marketing
Copy link
Contributor

@pkarman pkarman left a comment

Choose a reason for hiding this comment

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

+1

Copy link
Contributor

@jessieay jessieay left a comment

Choose a reason for hiding this comment

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

🆒 -- although it would also be cool if updating this only required a change in one place!

@zachmargolis
Copy link
Contributor Author

@jessieay it is only updating in one place? The rest are just specs?

@jessieay
Copy link
Contributor

jessieay commented Apr 5, 2017

@zachmargolis I was including specs in my count! I would incline to referencing the constant in specs (in general, I am down with referencing constants in specs rather than magic strings or numbers), but I think we already have this convo and I am cool with keeping as-is.

@monfresh
Copy link
Contributor

monfresh commented Apr 5, 2017

One way to address @jessieay's point is to move the string to a locale file. That way, the spec tests that the correct locale entry is being called, and changing the entry won't affect the tests, the same way we test most of our strings.

@zachmargolis
Copy link
Contributor Author

As @jessieay alluded to, I like testing like this because I want to make sure it's the right string. For example if we re-used a constant in the tests and there was a typo with a 4th 'w' in the constant place, then the tests wouldn't catch anything and we'd ship a bug. I think minimal duplication for string-building things like this is totally acceptable 😁

@zachmargolis zachmargolis merged commit 9393957 into master Apr 5, 2017
@zachmargolis zachmargolis deleted the margolis-use-www branch April 5, 2017 17:11
pkarman pushed a commit that referenced this pull request Apr 6, 2017
**Why**: We're making www. the canonical domain for marketing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants