Skip to content

feat(request-maker): use upstream httpsnippet - #663

Merged
marcelltoth merged 7 commits into
betafrom
feat/httpsnippet
Nov 19, 2020
Merged

feat(request-maker): use upstream httpsnippet#663
marcelltoth merged 7 commits into
betafrom
feat/httpsnippet

Conversation

@marcelltoth

Copy link
Copy Markdown
Contributor

Resolves#628

Depends on DefinitelyTyped/DefinitelyTyped#48987

After merging this PR we should archive stoplightio/httpsnippet

@marcelltoth
marcelltoth requested a review from a teamOctober 21, 2020 13:08
@marcelltothmarcelltoth self-assigned this Oct 21, 2020
@netlify

netlifyBot commented Oct 21, 2020

Copy link
Copy Markdown

Deploy preview for stoplight-elements ready!

Built with commit e3611f3

https://deploy-preview-663--stoplight-elements.netlify.app

@marcelltoth
marcelltoth requested a review from a teamOctober 28, 2020 12:59

@mallacharimallachari left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Works good!
There are TS problems regarding importing modules. Check out test step.

@marcelltoth

Copy link
Copy Markdown
ContributorAuthor

There are TS problems regarding importing modules. Check out test step.

I'm aware. There are some DefinitelyTyped issues: DefinitelyTyped/DefinitelyTyped#49170

Also we make quite a lot of lies in the toHAR method, it returns something that barely resembles a HAR object. Will fix that right after my DT HAR PR gets merged.

name: p.name,
fileName: p.name,
value: p.value,
value: p.value instanceof File ? '<file contents>' : p.value,

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These are intentionally dummy. I wanted to make the least intrusive change possible while correcting the type violations.

@marcelltoth
marcelltoth requested review from a team and mallachariOctober 30, 2020 16:26

@mallacharimallachari left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Still good to me 👍
There's some build error but it looks irrelevant. Locally works fine.

@mallachari

Copy link
Copy Markdown

Oh, the tests are failing. I think you forgot to add new snippet data

@marcelltoth

Copy link
Copy Markdown
ContributorAuthor

Tests fixed.

Unfortunately it looks like the build cannot be fixed just yet.
Kong did not publish a release that contains the fix (Kong/httpsnippet@0fc7f4a) yet. This is the master commit, was committed on Oct 6 but the latest release is September 25.

Iceboxing this until the upstream issue is resolved

@marcelltoth
marcelltoth marked this pull request as draft November 2, 2020 08:03
@marcelltoth
marcelltoth marked this pull request as ready for review November 19, 2020 13:24
@marcelltoth

Copy link
Copy Markdown
ContributorAuthor

I convinced Kong to release a new version 🎉 😇

So this is good to go. Rebased.

@marcelltoth
marcelltoth merged commit 5db3159 into betaNov 19, 2020
@marcelltoth
marcelltoth deleted the feat/httpsnippet branch November 19, 2020 14:51
Sign up for freeto 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.

Drop stoplightio/httpsnippet and use it's upstream instead

3 participants

@marcelltoth@mallachari@philsturgeon