Skip to content

profile.fprof mix task#3363

Merged
josevalim merged 4 commits intoelixir-lang:masterfrom
sasa1977:mix-fprof-support
Aug 11, 2015
Merged

profile.fprof mix task#3363
josevalim merged 4 commits intoelixir-lang:masterfrom
sasa1977:mix-fprof-support

Conversation

@sasa1977
Copy link
Contributor

Based on the discussion on the mailing list and further discussion on irc channel.

There's one known documentation issue I wasn't able to resolve (I'll add a note in the commit), but hopefully the pull's at least in the ballpark :-)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This header doesn't render properly in docs (leading spaces are dropped). Any idea how to work around this?

image

Copy link
Member

Choose a reason for hiding this comment

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

Add a # above Total just so it is rendered properly then. :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what you mean. Adding a # right above T preserves the spaces, but the # is visible and the line is rendered as a comment:

image

As a trick, I was thinking maybe adding a caption to the column, e.g. TITLE, or NAME, or something similar. Then the header starts at column 1, and we have no problems.

Copy link
Member

Choose a reason for hiding this comment

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

You can do a proper markdown table, check Kernel.Typespec for examples.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A table is an interesting workaround, although I'd prefer to have it as a standard code snippet (pre element) since this is the output of the task. But yeah, that might be one way of dealing with it. I could say, "The task produces the following table:" or something similar.

Copy link
Member

Choose a reason for hiding this comment

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

Just add the # at the top or any other character. We don't need to perfectly reflect the table, anything that looks sufficiently nice, will suffice.

Copy link
Member

Choose a reason for hiding this comment

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

Btw, the docs here are fantastic.

@josevalim
Copy link
Member

This is beautiful, thank you. Just a heads up, it will take some time for me to get back to you. I am likely not putting any dev cycles on Elixir in June (trying to get Plug+Ecto as 1.0 out).

@sasa1977
Copy link
Contributor Author

No hurry. Do you have any idea though why travis would fail on the unrelated code with +T 9 option set?

@ericmj
Copy link
Member

ericmj commented May 29, 2015

Travis failure is an unrelated race condition.

@sasa1977
Copy link
Contributor Author

Travis failure is an unrelated race condition.

I suspected as much. Should I just restart the build then?

@sasa1977
Copy link
Contributor Author

I fixed the doc table layout by using a fake space (see 08031e9). It looks ok both in web docs and terminal (mix help).

@ericmj
Copy link
Member

ericmj commented May 29, 2015

Although a nice hack and a good idea I don't think we should use NBSP. It's easy to accidentally break in the future and you don't know you broke until you look at the generated docs.

@josevalim
Copy link
Member

Yeah, what @ericmj said. :)

@sasa1977
Copy link
Contributor Author

So it's a # then?

@josevalim
Copy link
Member

Yeah, or anything simple like it.

José Valimwww.plataformatec.com.br
http://www.plataformatec.com.br/Founder and Lead Developer

@sasa1977
Copy link
Contributor Author

OK, I pushed the # char. Once you're ok with the complete feature, and before the merge, I could rebase/squash these commits to keep the history clean. No rush with merging, as far as I'm concerned.

josevalim added a commit that referenced this pull request Aug 11, 2015
@josevalim josevalim merged commit 82f800d into elixir-lang:master Aug 11, 2015
@josevalim
Copy link
Member

❤️ 💚 💙 💛 💜

@sasa1977 sasa1977 deleted the mix-fprof-support branch February 24, 2020 10:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants