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

Run rustfmt on everything #357

Merged
merged 2 commits into from Dec 17, 2018
Merged

Conversation

vimpunk
Copy link
Contributor

@vimpunk vimpunk commented Nov 20, 2018

I was reading through the code and noticed that the formatting was different from what I understand to be the currently agreed upon convention. So I ran rustfmt on the entire repo. I hope that's welcome, it made reading the code more pleasant. Tests run as previously.

data: NodeData::Text { ref contents },
..
}) = previous()
{
Copy link
Contributor Author

@vimpunk vimpunk Nov 20, 2018

Choose a reason for hiding this comment

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

I'm not happy about the placement of some of the opening brackets, like the one here. Maybe I could manually adjust these cases.


fn get_document(&mut self) -> usize {
0
}

fn get_template_contents(&mut self, target: &usize) -> usize {
if let Some(expanded_name!(html "template")) = self.names.get(&target).map(|n| n.expanded()) {
if let Some(expanded_name!(html "template")) = self.names.get(&target).map(|n| n.expanded())
Copy link
Contributor Author

@vimpunk vimpunk Nov 20, 2018

Choose a reason for hiding this comment

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

The call chain could probably be broken up into separate lines.

@nox
Copy link
Contributor

nox commented Nov 20, 2018

I don't think we should merge this PR if we don't enforce the same rules as servo/servo with the same settings.

@vimpunk
Copy link
Contributor Author

vimpunk commented Nov 20, 2018

@nox Oh, I missed that Servo has its own rusfmt config. I've added it to this project and rerun the formatter. What do you think?

@jdm
Copy link
Member

jdm commented Dec 13, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 822acf5 has been approved by jdm

@bors-servo
Copy link
Contributor

🔒 Merge conflict

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #347) made this pull request unmergeable. Please resolve the merge conflicts.

@Ygg01
Copy link
Contributor

Ygg01 commented Dec 17, 2018

@mandreyel It seems some updates broke this PR, would you like to retry this?

@vimpunk
Copy link
Contributor Author

vimpunk commented Dec 17, 2018

@Ygg01 Yes, sorry, I was a bit busy and forgot about this. I'll get to it tonight or tomorrow.

@Ygg01
Copy link
Contributor

Ygg01 commented Dec 17, 2018

Oh, no problem, I'm sorry you waited as long as you did.

@vimpunk
Copy link
Contributor Author

vimpunk commented Dec 17, 2018

Alright, done :)

@jdm
Copy link
Member

jdm commented Dec 17, 2018

@bors-servo r+

@bors-servo
Copy link
Contributor

📌 Commit 12b4ce0 has been approved by jdm

@bors-servo
Copy link
Contributor

⌛ Testing commit 12b4ce0 with merge 45b2fca...

bors-servo pushed a commit that referenced this pull request Dec 17, 2018
Run rustfmt on everything

I was reading through the code and noticed that the formatting was different from what I understand to be the currently agreed upon convention. So I ran rustfmt on the entire repo. I hope that's welcome, it made reading the code more pleasant. Tests run as previously.
@bors-servo
Copy link
Contributor

☀️ Test successful - status-travis
Approved by: jdm
Pushing 45b2fca to master...

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.

None yet

5 participants