Skip to content

/ servo

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

Jump to bottom

Use builder pattern with RequestInit #22427

Closed
jdm opened this issue Dec 11, 2018 · 6 comments
Closed

Use builder pattern with RequestInit #22427

jdm opened this issue Dec 11, 2018 · 6 comments
Labels
A-content/dom A-network C-assigned E-candidate-for-mentoring I-cleanup I-refactor

Comments

@jdm
Copy link
Member

@jdm jdm commented Dec 11, 2018

If RequestInit::new accepts all of the mandatory arguments and then the builder pattern is used for customizable options, the resulting code might be easier to match against specification text like https://html.spec.whatwg.org/multipage/media.html#media-elements.
@lucasfantacuci
Copy link
Contributor

@lucasfantacuci lucasfantacuci commented Dec 13, 2018 •
edited

It could be a great way to learn the Builder pattern in Rust for me.

Can I work on it?
@jdm
Copy link
Member Author

@jdm jdm commented Dec 13, 2018

Sure! Look for uses of RequestInit { in components/ and those are the ones that we will want to replace with a constructor and builder methods.
@lucasfantacuci
Copy link
Contributor

@lucasfantacuci lucasfantacuci commented Dec 13, 2018

@highfive: assign me
@highfive highfive added the C-assigned label Dec 13, 2018
@highfive
Copy link

@highfive highfive commented Dec 13, 2018

Hey @lucasfantacuci! Thanks for your interest in working on this issue. It's now assigned to you!
@lucasfantacuci lucasfantacuci mentioned this issue Dec 21, 2018
Merged
4 of 4 tasks complete
@lucasfantacuci
Copy link
Contributor

@lucasfantacuci lucasfantacuci commented Dec 21, 2018

@jdm I was analyzing the code from components and I couldn't realize what are all the mandatory arguments. I can't be sure.
@jdm
Copy link
Member Author

@jdm jdm commented Dec 21, 2018

I recommend starting from the specification definition of a request (https://fetch.spec.whatwg.org/#concept-request) - anything that says "unless otherwise set" or "unless stated otherwise" is optional, so anything without that is mandatory. Then try to match the mandatory ones against the members of RequestInit.
bors-servo added a commit that referenced this issue Apr 5, 2019
@bors-servo
Auto merge of #22521 - lucasfantacuci:use_build_pattern_with_requesti…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
af4afb5 
…nit, r=<try>

[WIP]Refactoring RequestInit to use a Builder Pattern

<!-- Please describe your changes on the following line: -->

If RequestInit::new accepts all of the mandatory arguments and then the builder pattern is used for customizable options, the resulting code might be easier to match against specification text like

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22427
- [x ] These changes do not require tests because it is a code refactoring.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22521)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Apr 9, 2019
@bors-servo
Auto merge of #22521 - lucasfantacuci:use_build_pattern_with_requesti…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
4f5cd94 
…nit, r=jdm

Refactoring RequestInit to use a Builder Pattern

<!-- Please describe your changes on the following line: -->

If RequestInit::new accepts all of the mandatory arguments and then the builder pattern is used for customizable options, the resulting code might be easier to match against specification text like

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22427
- [x] These changes do not require tests because it is a code refactoring.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22521)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Apr 10, 2019
@bors-servo
Auto merge of #22521 - lucasfantacuci:use_build_pattern_with_requesti…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
5ea2366 
…nit, r=jdm

Refactoring RequestInit to use a Builder Pattern

<!-- Please describe your changes on the following line: -->

If RequestInit::new accepts all of the mandatory arguments and then the builder pattern is used for customizable options, the resulting code might be easier to match against specification text like

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22427
- [x] These changes do not require tests because it is a code refactoring.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22521)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Apr 10, 2019
@bors-servo
Auto merge of #22521 - lucasfantacuci:use_build_pattern_with_requesti…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
5370151 
…nit, r=jdm

Refactoring RequestInit to use a Builder Pattern

<!-- Please describe your changes on the following line: -->

If RequestInit::new accepts all of the mandatory arguments and then the builder pattern is used for customizable options, the resulting code might be easier to match against specification text like

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22427
- [x] These changes do not require tests because it is a code refactoring.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22521)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Apr 10, 2019
@bors-servo
Auto merge of #22521 - lucasfantacuci:use_build_pattern_with_requesti…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
cb5dce5 
…nit, r=jdm,KiChjang

Refactoring RequestInit to use a Builder Pattern

<!-- Please describe your changes on the following line: -->

If RequestInit::new accepts all of the mandatory arguments and then the builder pattern is used for customizable options, the resulting code might be easier to match against specification text like

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22427
- [x] These changes do not require tests because it is a code refactoring.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22521)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Apr 11, 2019
@bors-servo
Auto merge of #22521 - lucasfantacuci:use_build_pattern_with_requesti…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
f14d199 
…nit, r=jdm,KiChjang

Refactoring RequestInit to use a Builder Pattern

<!-- Please describe your changes on the following line: -->

If RequestInit::new accepts all of the mandatory arguments and then the builder pattern is used for customizable options, the resulting code might be easier to match against specification text like

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22427
- [x] These changes do not require tests because it is a code refactoring.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22521)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this issue Apr 11, 2019
@bors-servo
Auto merge of #22521 - lucasfantacuci:use_build_pattern_with_requesti…
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
9ab0af0 
…nit, r=jdm,KiChjang

Refactoring RequestInit to use a Builder Pattern

<!-- Please describe your changes on the following line: -->

If RequestInit::new accepts all of the mandatory arguments and then the builder pattern is used for customizable options, the resulting code might be easier to match against specification text like

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #22427
- [x] These changes do not require tests because it is a code refactoring.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22521)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-content/dom A-network C-assigned E-candidate-for-mentoring I-cleanup I-refactor
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Refactoring RequestInit to use a Builder Pattern
3 participants
@jdm @highfive @lucasfantacuci
You can’t perform that action at this time.