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

Add HTTP handler #137

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Add HTTP handler #137

wants to merge 4 commits into from

Conversation

rmccue
Copy link
Member

@rmccue rmccue commented May 27, 2020

This allows use via API Gateway in a web form, powering https://make.hmn.md/lint-check/

Requires API Gateway reconfiguration; hm-linter-development has that reconfiguration, but hm-linter does not yet. (Also, I've already deployed this to hm-linter-development.)

Sidenote: I spent a few hours trying to get API Gateway testing locally, and could not work it out, I don't think it's really possible. For testing this, unfortunately the best way is to use hm-linter-development so you get the actual event object from API Gateway. 😞

This allows use via API Gateway in a web form.
@rmccue rmccue marked this pull request as ready for review May 27, 2020 17:02
@rmccue rmccue requested a review from mikeselander May 27, 2020 17:02
Copy link
Contributor

@mikeselander mikeselander left a comment

Choose a reason for hiding this comment

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

This is pretty simple and I like how you handled the separate request 👍

Once stylelint is enabled this will be. good to go.

src/http.js Outdated
version: 'inherit',
},
stylelint: {
enabled: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since there's a CSS button on the Make site, we'll need to enable stylelint here.

Copy link
Member Author

Choose a reason for hiding this comment

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

duh! I copied this from config.js but that was pre-merge!


// Then, prepare the linters.
const config = {
version: data.get( 'version' ) || 'latest',
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a future enhancement, but it would be really great to be able to push up a "future" version with the unreleased changes and check against them before they're released. I think it would help us in testing and potentially help projects with checking some code without having to update a. whole project.

Copy link
Member Author

Choose a reason for hiding this comment

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

Should be possible already I think?

let baseDir = dir;
while ( fnParts.length > 1 ) {
const nextPart = fnParts.shift();
if ( nextPart === '..' || nextPart === '.' ) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nicely handled 👌

@rmccue rmccue requested a review from mikeselander May 29, 2020 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants