Add BodyMultipart

[earthboundkid]

No description provided.

[coveralls]

Pull Request Test Coverage Report for Build 11958952365

Details

• 11 of 20 (55.0%) changed or added relevant lines in 1 file are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage decreased (-0.8%) to 90.779%

---

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
config.go	11	20	55.0%

Totals
Change from base Build 10584845958:	-0.8%
Covered Lines:	827
Relevant Lines:	911

---

💛 - Coveralls

[Crocmagnon]

Nice! 🎉

Would it be possible to make it available as well as a method directly on *Builder if that makes sense? I imagined something like this, similar to the Body* methods:

requests.URL("https://example.com").
    BodyMultipart("boundary", func(multi *multipart.Writer) error {
        // do something with multi
        return nil
    }).
    Fetch(ctx)

[earthboundkid]

Do you think there's that much demand for it? OTOH, nesting the Config calls looks pretty ugly. Also unlike some of the other stuff I'm trying to move out to separate packages, this imports mime/multipart, which net/http also imports, so it's not introducing any import dependencies. 🤔

[earthboundkid]

There's some weirdness around the random boundary. I think if you created a Builder and reused it, you would get the same "random" boundary every time until you make a new Builder. That's probably an okay limitation for a Config, but for a method I would expect it to work every time…

[Crocmagnon]

Do you think there's that much demand for it?

Probably not, seeing the activity about the topic in the discussion 😛.
My concern is mostly about discoverability, I wasn't aware of Config before your PR.
For this kind of "simple" HTTP usage, I'd be in favor of consistency in the API.

nesting the Config calls looks pretty ugly.

Agreed.

I think if you created a Builder and reused it, you would get the same "random" boundary every time until you make a new Builder.

Is it an issue? If the generated boundary is sufficiently random and is already OK for a single usage, I'm guessing that it can be safely reused. There's no requirement for it to change on every request as far as I'm aware of.

[Crocmagnon]

Hello! Any blocker here? Can I help releasing this? :)

[earthboundkid]

This looks pretty good. I think it can be released as just a config for now and we can decide if it should be promoted to a method later. I want to change the example test to use a content type for en.txt, but other than that it looks good to go.