This repository has been archived by the owner on May 9, 2021. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 492
Check for redundant if err != nil constructs. #319
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Detect and complain about constructs like: if err := foo(); err != nil { return err } return nil (Issue golang#312)
Also don't emit lint errors if there are any comments explaining the construct (between if and return statements).
alandonovan
approved these changes
Sep 18, 2017
philipnrmn
added a commit
to mesosphere-backup/dcos-metrics
that referenced
this pull request
Sep 19, 2017
In scripts/test.sh we naively run go get to update golint. A new rule has just been added to golint (golang/lint#319) and I don't want to complicate my current PR (#118). I will revert this commit, fix any outstanding nil-returns and consider pinning golint in a future PR.
philipnrmn
added a commit
to mesosphere-backup/dcos-metrics
that referenced
this pull request
Sep 19, 2017
* Export TaskInfo from the mesos agent collector We will reference TaskInfo in a map of container IDs to task info in a later commit. * Add a struct for container->task relationships ContainerTaskRels has a single member, a map of container IDs to TaskInfo references. It has a getter and a setter for easy access threadsafe access to the relationship map, and an update method which populates it from agentState. * Return task_id and task_name in /app output The new ContainerTaskRels argument providers access to the task metadata for each container. * Add tests for extracting a record with task data We add a new mock object with appropriate metadata and supply it to avroRecord.extract(), checking that the task IDs in the ContainerTaskRels are the same ones that correspond to the container ID. * Add tests for executor ID and framework ID This is orthogonal to the intent of this PR, however it's a small change and seemed like a quick win. * Refer to ContainerTaskRels in Framework collector This reference can be provided to the extract method from the framework collector. * Pass ref to ContainerTaskRels into transform * Update task mapping from agent collector We update the task mapping after every new fetch from agent state. * Pass reference to mapping into framework collector Now the framework collector has access to the ContainerTaskRels object which belongs to the mesos agent collector. * Clarify arg name * Defer unlock for consistency * Temporarily skip golint in build In scripts/test.sh we naively run go get to update golint. A new rule has just been added to golint (golang/lint#319) and I don't want to complicate my current PR (#118). I will revert this commit, fix any outstanding nil-returns and consider pinning golint in a future PR. * Create new TaskInfo object to avoid memory issues * Create ContainerTaskRels in dcos-metrics.go Rather than initiating ContainerTaskRels in the framework collector, we now initiate it in dcos-metrics.go and pass a reference into each collector instead. * Initiate ContainerTaskRels with a utility method * Remove unnecessary nil check
This was referenced Sep 19, 2017
philipnrmn
added a commit
to mesosphere-backup/dcos-metrics
that referenced
this pull request
Sep 19, 2017
* Export TaskInfo from the mesos agent collector We will reference TaskInfo in a map of container IDs to task info in a later commit. * Add a struct for container->task relationships ContainerTaskRels has a single member, a map of container IDs to TaskInfo references. It has a getter and a setter for easy access threadsafe access to the relationship map, and an update method which populates it from agentState. * Return task_id and task_name in /app output The new ContainerTaskRels argument providers access to the task metadata for each container. * Add tests for extracting a record with task data We add a new mock object with appropriate metadata and supply it to avroRecord.extract(), checking that the task IDs in the ContainerTaskRels are the same ones that correspond to the container ID. * Add tests for executor ID and framework ID This is orthogonal to the intent of this PR, however it's a small change and seemed like a quick win. * Refer to ContainerTaskRels in Framework collector This reference can be provided to the extract method from the framework collector. * Pass ref to ContainerTaskRels into transform * Update task mapping from agent collector We update the task mapping after every new fetch from agent state. * Pass reference to mapping into framework collector Now the framework collector has access to the ContainerTaskRels object which belongs to the mesos agent collector. * Clarify arg name * Defer unlock for consistency * Temporarily skip golint in build In scripts/test.sh we naively run go get to update golint. A new rule has just been added to golint (golang/lint#319) and I don't want to complicate my current PR (#118). I will revert this commit, fix any outstanding nil-returns and consider pinning golint in a future PR. * Create new TaskInfo object to avoid memory issues * Create ContainerTaskRels in dcos-metrics.go Rather than initiating ContainerTaskRels in the framework collector, we now initiate it in dcos-metrics.go and pass a reference into each collector instead. * Initiate ContainerTaskRels with a utility method * Remove unnecessary nil check
philipnrmn
added a commit
to mesosphere-backup/dcos-metrics
that referenced
this pull request
Sep 19, 2017
* Export TaskInfo from the mesos agent collector We will reference TaskInfo in a map of container IDs to task info in a later commit. * Add a struct for container->task relationships ContainerTaskRels has a single member, a map of container IDs to TaskInfo references. It has a getter and a setter for easy access threadsafe access to the relationship map, and an update method which populates it from agentState. * Return task_id and task_name in /app output The new ContainerTaskRels argument providers access to the task metadata for each container. * Add tests for extracting a record with task data We add a new mock object with appropriate metadata and supply it to avroRecord.extract(), checking that the task IDs in the ContainerTaskRels are the same ones that correspond to the container ID. * Add tests for executor ID and framework ID This is orthogonal to the intent of this PR, however it's a small change and seemed like a quick win. * Refer to ContainerTaskRels in Framework collector This reference can be provided to the extract method from the framework collector. * Pass ref to ContainerTaskRels into transform * Update task mapping from agent collector We update the task mapping after every new fetch from agent state. * Pass reference to mapping into framework collector Now the framework collector has access to the ContainerTaskRels object which belongs to the mesos agent collector. * Clarify arg name * Defer unlock for consistency * Temporarily skip golint in build In scripts/test.sh we naively run go get to update golint. A new rule has just been added to golint (golang/lint#319) and I don't want to complicate my current PR (#118). I will revert this commit, fix any outstanding nil-returns and consider pinning golint in a future PR. * Create new TaskInfo object to avoid memory issues * Create ContainerTaskRels in dcos-metrics.go Rather than initiating ContainerTaskRels in the framework collector, we now initiate it in dcos-metrics.go and pass a reference into each collector instead. * Initiate ContainerTaskRels with a utility method * Remove unnecessary nil check
philipnrmn
added a commit
to mesosphere-backup/dcos-metrics
that referenced
this pull request
Sep 19, 2017
* Export TaskInfo from the mesos agent collector We will reference TaskInfo in a map of container IDs to task info in a later commit. * Add a struct for container->task relationships ContainerTaskRels has a single member, a map of container IDs to TaskInfo references. It has a getter and a setter for easy access threadsafe access to the relationship map, and an update method which populates it from agentState. * Return task_id and task_name in /app output The new ContainerTaskRels argument providers access to the task metadata for each container. * Add tests for extracting a record with task data We add a new mock object with appropriate metadata and supply it to avroRecord.extract(), checking that the task IDs in the ContainerTaskRels are the same ones that correspond to the container ID. * Add tests for executor ID and framework ID This is orthogonal to the intent of this PR, however it's a small change and seemed like a quick win. * Refer to ContainerTaskRels in Framework collector This reference can be provided to the extract method from the framework collector. * Pass ref to ContainerTaskRels into transform * Update task mapping from agent collector We update the task mapping after every new fetch from agent state. * Pass reference to mapping into framework collector Now the framework collector has access to the ContainerTaskRels object which belongs to the mesos agent collector. * Clarify arg name * Defer unlock for consistency * Temporarily skip golint in build In scripts/test.sh we naively run go get to update golint. A new rule has just been added to golint (golang/lint#319) and I don't want to complicate my current PR (#118). I will revert this commit, fix any outstanding nil-returns and consider pinning golint in a future PR. * Create new TaskInfo object to avoid memory issues * Create ContainerTaskRels in dcos-metrics.go Rather than initiating ContainerTaskRels in the framework collector, we now initiate it in dcos-metrics.go and pass a reference into each collector instead. * Initiate ContainerTaskRels with a utility method * Remove unnecessary nil check
This was referenced Sep 25, 2017
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Detect and complain about constructs like:
This is issue #312