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

Improves examples copy&paste-ability #97

Closed
wants to merge 1 commit into from
Closed

Improves examples copy&paste-ability #97

wants to merge 1 commit into from

Conversation

neykov
Copy link
Member

@neykov neykov commented Jul 25, 2016

Remove $ from line beginnings, so that the example can easily be copy&pasted in the console.
Also fix tomcat port so that the example works out of the box.

@neykov neykov changed the title Improves examples copy&pate-ability Improves examples copy&paste-ability Jul 25, 2016
@drigodwin
Copy link
Member

LGTM 👍

@@ -435,14 +435,14 @@ you could use a load generator like jmeter, or use a script such as the one show
(changing URL for the URL of your load-balancer):

{% highlight bash %}
$ URL=http://10.10.10.101:8000/
$ URL=http://10.10.10.101:8080/
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought nginx would come up on 8000, rather than 8080 (which is the port that the first tomcat would use).

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point, will revert.

@ahgittin
Copy link
Contributor

ahgittin commented Nov 7, 2016

i'd prefer a clever solution to avoiding the $ rather than removing it from the display. it is used throughout to distinguish what is entered and multi-line from what is returned.

@neykov
Copy link
Member Author

neykov commented Nov 7, 2016

The change is not removing all $s - it's leaving the one at the command start. The removed characters are in the following lines in a multi-line command.
See here for an example of what its trying to fix http://brooklyn.apache.org/v/latest/start/policies.html#auto-scaling.
The example snippet looks confusing with all those $ on each line (that's a multi-line command). Messing with copy-pasteability is an additional problem.

@ahgittin
Copy link
Contributor

ahgittin commented Nov 7, 2016

ah of course. should we render it as:

$ for i in byon{1..4}; do
>   vagrant ssh ${i} --command 'ps aux | grep -i tomcat |  grep -v grep | awk '\''{print $2}'\'' | xargs kill -9'
> done

?

or i'd say to remove the leading char altogether

@neykov
Copy link
Member Author

neykov commented Nov 7, 2016

I'm for the latter so it's easier to just grab it from the snippet and paste it somewhere. Otherwise you'd have the same problem as now - need to paste it somewhere intermediate to remove all those extra characters.

@ahgittin
Copy link
Contributor

ahgittin commented Nov 7, 2016

@neykov if you use the clipboard icon in the corner it does that for you, though i agree if you don't know about it it's irritating -- trying to fix that

@neykov
Copy link
Member Author

neykov commented Nov 7, 2016

The clipboard icon doesn't work on flash-free browsers so better not to depend on it doing the stripping.

@ahgittin
Copy link
Contributor

ahgittin commented Nov 7, 2016

+1

ahgittin added a commit to ahgittin/brooklyn-docs that referenced this pull request Nov 7, 2016
@ahgittin ahgittin mentioned this pull request Nov 7, 2016
@ahgittin
Copy link
Contributor

ahgittin commented Nov 7, 2016

@neykov is #123 a better approach?
no strong feelings if you prefer to remove the > multi-line prompt IE layer this on top of that

i suspect the -saefafefj flags to curl is a mistake which should be removed (and grep for it, i've noticed it in another file too) but want to confirm just in case it's a magic incantation on another curl impl /cc @grkvlt @Graeme-Miller

@neykov
Copy link
Member Author

neykov commented Nov 7, 2016

Closing in favour of #123.
Good spot on the curl argument agree should be safe to remove.

@neykov neykov closed this Nov 7, 2016
@neykov neykov deleted the copy-examples branch November 7, 2016 15:56
@ahgittin ahgittin mentioned this pull request Nov 8, 2016
asfgit pushed a commit that referenced this pull request Nov 17, 2016
tidy curl args

as discussed in #97 -- the old args looked really strange, at least on the os x `curl`.

would appreciate feedback from any of @drigodwin @duncangrant @Graeme-Miller @aledsage @grkvlt who might have some insight as to whether the old args were there for a reason.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
5 participants