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

[Website] Add last modified header #199

Open
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@drewfreyling
Copy link
Member

commented Oct 15, 2017

No description provided.

@PeterDaveHello
Copy link
Member

left a comment

I believe at least .editorconfig and .gitignore update should be separated in another PR/commit, agree?

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Oct 15, 2017

Agreed.

@PeterDaveHello

This comment has been minimized.

Copy link
Member

commented Oct 16, 2017

The tricky point is the header for library page, we don't have timestamp on library update yet. It's not so suitable to directly apply packages.min.json modified date on it.

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Oct 16, 2017

@drewfreyling drewfreyling force-pushed the drewfreyling:feature/lastmod branch from ac4c7dd to 22b11fc Dec 28, 2017

@drewfreyling drewfreyling force-pushed the drewfreyling:feature/lastmod branch from 22b11fc to bfcf131 Dec 28, 2017

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Dec 28, 2017

@PeterDaveHello i've removed the extra files and removed the libraries page from the cache validity check.

@PeterDaveHello PeterDaveHello force-pushed the cdnjs:master branch from 5d70a78 to 4a73ca7 Apr 18, 2018

@MattIPv4

This comment has been minimized.

Copy link
Member

commented Apr 13, 2019

@drewfreyling Hey 👋 Could you please resolve the conflicts and then I can chase this being merged?

@MattIPv4 MattIPv4 changed the title Add last modified header. [Website] Add last modified header Apr 13, 2019

@drewfreyling drewfreyling requested a review from MattIPv4 Apr 18, 2019

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Apr 18, 2019

I'll get it fixed it up.

@MattIPv4
Copy link
Member

left a comment

This looks good to me.

@MattIPv4

This comment has been minimized.

Copy link
Member

commented Apr 25, 2019

@PeterDaveHello Can you review this please?

@MattIPv4 MattIPv4 requested a review from PeterDaveHello Apr 25, 2019

@MattIPv4
Copy link
Member

left a comment

As Peter appears to be rather busy, I thought I should do an in-depth review of the code style.

Show resolved Hide resolved webServer.js Outdated
Show resolved Hide resolved webServer.js Outdated
Show resolved Hide resolved webServer.js Outdated
Show resolved Hide resolved webServer.js Outdated
Show resolved Hide resolved webServer.js Outdated
Show resolved Hide resolved webServer.js Outdated

@drewfreyling drewfreyling self-assigned this May 28, 2019

@PeterDaveHello PeterDaveHello removed their request for review Jun 1, 2019

@PeterDaveHello

This comment has been minimized.

Copy link
Member

commented Jun 1, 2019

Will review again once above issues resolved ;)

@drewfreyling drewfreyling requested a review from PeterDaveHello Jun 4, 2019

Show resolved Hide resolved webServer.js Outdated
Show resolved Hide resolved webServer.js
@MattIPv4
Copy link
Member

left a comment

Lgtm.

@PeterDaveHello
Copy link
Member

left a comment

Something broken?

TypeError: Cannot read property 'content' of undefined
    at generatePage (/home/peter/new-website/webServer.js:136:33)
    at /home/peter/new-website/webServer.js:573:26
    at Layer.handle_error (/home/peter/new-website/node_modules/express/lib/router/layer.js:71:5)
    at trim_prefix (/home/peter/new-website/node_modules/express/lib/router/index.js:315:13)
    at /home/peter/new-website/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/home/peter/new-website/node_modules/express/lib/router/index.js:335:12)
    at next (/home/peter/new-website/node_modules/express/lib/router/index.js:275:10)
    at Layer.handle_error (/home/peter/new-website/node_modules/express/lib/router/layer.js:67:12)
    at trim_prefix (/home/peter/new-website/node_modules/express/lib/router/index.js:315:13)
    at /home/peter/new-website/node_modules/express/lib/router/index.js:284:7
@MattIPv4

This comment has been minimized.

Copy link
Member

commented Jun 12, 2019

cc #157

@drewfreyling

This comment has been minimized.

Copy link
Member Author

commented Jun 19, 2019

Something broken?

TypeError: Cannot read property 'content' of undefined
    at generatePage (/home/peter/new-website/webServer.js:136:33)
    at /home/peter/new-website/webServer.js:573:26
    at Layer.handle_error (/home/peter/new-website/node_modules/express/lib/router/layer.js:71:5)
    at trim_prefix (/home/peter/new-website/node_modules/express/lib/router/index.js:315:13)
    at /home/peter/new-website/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/home/peter/new-website/node_modules/express/lib/router/index.js:335:12)
    at next (/home/peter/new-website/node_modules/express/lib/router/index.js:275:10)
    at Layer.handle_error (/home/peter/new-website/node_modules/express/lib/router/layer.js:67:12)
    at trim_prefix (/home/peter/new-website/node_modules/express/lib/router/index.js:315:13)
    at /home/peter/new-website/node_modules/express/lib/router/index.js:284:7

It might be from the rebase, I'll investigate.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.