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

Jq/0.7 #198

Merged
merged 2 commits into from
Feb 13, 2018
Merged

Jq/0.7 #198

merged 2 commits into from
Feb 13, 2018

Conversation

quinnj
Copy link
Member

@quinnj quinnj commented Feb 13, 2018

No description provided.

@quinnj quinnj merged commit b7b8e0f into master Feb 13, 2018
@@ -107,7 +107,7 @@ using JSON

#=
@sync begin
io = BufferStream()
io = Base.BufferStream()
Copy link
Contributor

Choose a reason for hiding this comment

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

That's annoying ;)
See: JuliaLang/julia#25802 (comment)

sockettype(url::URI, default) = url.scheme in ("wss", "https") ? SSLContext :
default

sockettype(url::URI, default) = url.scheme in ("wss", "https") ? SSLContext : default
Copy link
Contributor

Choose a reason for hiding this comment

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

This might seem like weird formatting on a wide terminal, but I have taken some care to keep lines of code less than 80 columns where possible.

function parse_header_field(bytes::SubString{String})::Tuple{Header,
SubString{String}}

function parse_header_field(bytes::SubString{String})::Tuple{Header,SubString{String}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Was wrapped to be < 80 cols.

@samoconnor
Copy link
Contributor

Hi @quinnj,
The removal of the blank line between a docstring and the statement that it documents seems like a good change. I don't know why I started doing that, but I seem to have done a lot of it :)
However, most of the other removed whitespace is in places that I intended it to communicate separation of concerns in the source text. e.g.

The intention is to provide visual cues to the reader, that while not consciously noticed, will help the skimming eye to pick out the first words of a heading, or the start of a paragraph (group of methods).
I realise that this is all arguably subjective and down to aesthetic preference. If you feel strongly that to your eye the code was looking messy with the extra whitespace, then lets leave it as is.

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