-
Notifications
You must be signed in to change notification settings - Fork 6.3k
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
Get data from google doc for Announcements #1332
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two little nits, otherwise LGTM. 👍
Sorry for the delayed review!
layouts/announcements.hbs
Outdated
</div> | ||
</div> | ||
<script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/tabletop.js/1.4.2/tabletop.min.js"></script> | ||
<script type="text/javascript"> | ||
var public_spreadsheet_url = 'https://docs.google.com/spreadsheets/d/1xwQ6ciZLADL43My_XBIWLVRuDMl8cx_omYd1FRkBZ58/pubhtml'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use the standard.js style across the nodejs.org website, please make sure client side scripts follow the same style.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was copied from layouts/in-the-news.hbs#L22 to use layout as news sections. If something goes wrong I'll fix it on both pages.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pierreneter The code below now has semicolons at the end, for example, which is not the case for in-the-news.hbs
but had other issues fixed in #1352.
Could you please fix that so it validates against standard style?
See https://standardjs.com/demo.html for a quick check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. I will push a commit tonight
layouts/in-the-news.hbs
Outdated
@@ -25,16 +26,16 @@ | |||
window.onload = function () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use the standard.js style across the nodejs.org website, please make sure client side scripts follow the same style.
layouts/announcements.hbs
Outdated
</div> | ||
</div> | ||
<script type="text/javascript" src="https://cdnjs.cloudflare.com/ajax/libs/tabletop.js/1.4.2/tabletop.min.js"></script> | ||
<script type="text/javascript"> | ||
var public_spreadsheet_url = 'https://docs.google.com/spreadsheets/d/1xwQ6ciZLADL43My_XBIWLVRuDMl8cx_omYd1FRkBZ58/pubhtml'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pierreneter The code below now has semicolons at the end, for example, which is not the case for in-the-news.hbs
but had other issues fixed in #1352.
Could you please fix that so it validates against standard style?
See https://standardjs.com/demo.html for a quick check.
Can you also please remove yarn.lock? We already have package-lock.json or do we want to keep both? |
@fhemberger @lpinca all fixed, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for my shortcoming, thank you.
Great, thank you! |
* 'master' of https://github.com/nodejs/nodejs.org: (22 commits) Add Myles as TSC Director (nodejs#1365) board: add Myles Borins (nodejs#1362) doc: fix some links in guides (nodejs#1363) blog: update 8.5.0 release blog: v8.5.0 release post (nodejs#1360) Update board.md (nodejs#1359) Merge Website WG Members and Collaborators 🎉 (nodejs#1357) Updated it translation for index.md (nodejs#1353) Get data from google doc for Announcements (nodejs#1332) fix silver section head and paypal location (nodejs#1355) layouts/in-the-news.hbs: Use JS standard style (nodejs#1352) blog: release post for v6.11.3 Change member class (nodejs#1350) Fix wrongs Language Names in Working Groups page. (nodejs#1347) Remove current gold director (nodejs#1348) Fix instruction in dockerization guide. Issue nodejs#1344 (nodejs#1346) Update TSC member list (nodejs#1343) Update site.json of zh-cn locale (nodejs#1339) Update Silver member Directors for August 2017 (nodejs#1341) blog: add release post for v8.4.0 (nodejs#1340) ...
Enable show data from Google Doc.
close #1327