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

nsqadmin: Disambiguating Table Headings in Channels #184

Merged
merged 1 commit into from
Apr 25, 2013
Merged

nsqadmin: Disambiguating Table Headings in Channels #184

merged 1 commit into from
Apr 25, 2013

Conversation

elubow
Copy link
Contributor

@elubow elubow commented Apr 25, 2013

Added headers on top of the table headers to make it clear that the first 4 columns are message guages and the last 4 (or 5) columns are for stats. Let the naming argument commence.

@@ -143,6 +155,9 @@
{{else}}
<table class="table table-bordered table-condensed">
<tr>
<
Copy link
Member

Choose a reason for hiding this comment

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

stray?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I initially thought that line was a stray too but it doesn't appear to be. It looks like some Git weirdness.

Copy link
Member

Choose a reason for hiding this comment

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

remove it?

Copy link
Member

Choose a reason for hiding this comment

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

I dont know what you're talking about, see screenshot:

Screen Shot 2013-04-25 at 11 40 16 AM

Copy link
Member

Choose a reason for hiding this comment

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

actually, looks like that whole <tr> in the screenshot isn't needed

@mreiferson
Copy link
Member

I ran it locally and it looks good, I think it will be helpful for clarity.

cc @jehiah for naming

<thead>
<tr>
<th>&nbsp;</th>
<th colspan=4 style='text-align:center;'>Message Gauges</th>
Copy link
Member

Choose a reason for hiding this comment

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

For clarity, let's rename this Message Queue, and remove Message Queue from the <h4> heading above this table. All four numbers here represent actual messages outstanding so that title makes more sense scoped to these columns.

html style comments; can you quote the colspan attribute, and instead of using an inline style tag here set class="text-center". That class won't do anything yet, but i'll separately upgrade to the current bootstrap version to pick that up.

@elubow
Copy link
Contributor Author

elubow commented Apr 25, 2013

For some reason I had to git pull and it showed up. Git confuses me sometimes.

@mreiferson
Copy link
Member

now one more git magic trick... can you squash these down?

@mreiferson
Copy link
Member

we don't just push random commits haphazardly to master like @devdazed

😄 👊 🔥 🚒 🎆

@elubow
Copy link
Contributor Author

elubow commented Apr 25, 2013

Do you guys wonder why there aren't more contributors?

mreiferson added a commit that referenced this pull request Apr 25, 2013
nsqadmin: Disambiguating Table Headings in Channels
@mreiferson mreiferson merged commit 2f1e4f1 into nsqio:master Apr 25, 2013
absolute8511 pushed a commit to absolute8511/nsq that referenced this pull request Jan 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants