Skip to content

Add support for Insight Agent NG - #40

Closed
benjaminleesmith wants to merge 4 commits into
masterfrom
insight_agent_ng
Closed

Add support for Insight Agent NG#40
benjaminleesmith wants to merge 4 commits into
masterfrom
insight_agent_ng

Conversation

@benjaminleesmith

Copy link
Copy Markdown

No description provided.

Christopher Frost and others added 4 commits February 26, 2014 16:04
@cfdreddbot

Copy link
Copy Markdown

Hey benjaminleesmith!

Thanks for submitting this pull request!

All pull request authors must have a Contributor License Agreement (CLA) on-file with us. Please sign the appropriate CLA (individual or corporate).

When sending signed CLA please provide your github username in case of individual CLA or the list of github usernames that can make pull requests on behalf of your organization.

@benjaminleesmith

Copy link
Copy Markdown
Author

@tammersaleh

Copy link
Copy Markdown

 Code Climate has analyzed this pull request.

@nebhale

nebhale commented Mar 17, 2014

Copy link
Copy Markdown
Contributor

@benjaminleesmith A couple of general comments and I'll add more detailed ones in-line in the code.

First, please review the contributor guidelines and update the code. Ideally, there should be a single squashed commit with a more descriptive commit message (take a look at some others for examples). As well, the code should be re-based on top of the latest master commit before re-submitting. A git rebase --interactive <UPSTREAM-REMOTE-NAME>/master should do wonders for this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

What is the point of this change? I could understand something like /[Ii]nsight/, but I'm not sure what the point of the ^p is.

@nebhale

nebhale commented Mar 18, 2014

Copy link
Copy Markdown
Contributor

The stub-insight-agent file is 36M (!!). That doesn't sound like a stub to me. Anything larger than 1-2K must be trimmed down.

@nebhale nebhale closed this Mar 19, 2014
@nebhale
nebhale deleted the insight_agent_ng branch March 19, 2014 08:07
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.

5 participants