Skip to content
This repository was archived by the owner on May 9, 2021. It is now read-only.

Suggest io.SeekXXX instead of os.SEEK_XXX #302

Closed

Conversation

pravj
Copy link

@pravj pravj commented Jun 9, 2017

Implement a new lint function to suggest new constant replacing a depreciated constant.

os.SEEK_SET => io.SeekStart
os.SEEK_CUR => io.SeekCurrent
os.SEEK_END => io.SeekEnd

Fixes #294

Hi! @shurcooL @dsnet
Can you please review it?

Implement a new lint function to suggest new constant replacing
a depreciated constant.

os.SEEK_SET => io.SeekStart
os.SEEK_CUR => io.SeekCurrent
os.SEEK_END => io.SeekEnd

Fixes golang#294
@pravj
Copy link
Author

pravj commented Jun 9, 2017

Also, it's missing the error reporting convention with line number, I'm working on it.

Copy link
Member

@dsnet dsnet left a comment

Choose a reason for hiding this comment

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

Change looks fine overall. Please make this one small change.

lint.go Outdated
@@ -210,6 +210,7 @@ func (f *file) lint() {
f.lintTimeNames()
f.lintContextKeyTypes()
f.lintContextArgs()
f.lintDepreciatedConstants()
Copy link
Member

Choose a reason for hiding this comment

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

Rename Depreciated as Deprecated.

Depreciate means to lower in value. Deprecated means to express disapproval of.

Copy link
Author

Choose a reason for hiding this comment

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

Sure thing. Updated with #e8b641

@dmitshur
Copy link
Member

dmitshur commented Jul 1, 2017

Friendly ping @dsnet, the small change you requested has been done, what's next?

}

// lintDeprecatedConstants checks for the use of a deprecated constant
// and suggest a different constant in replacement
Copy link
Member

Choose a reason for hiding this comment

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

s/suggest/suggests

Also comments should have a period at the end (was lint run on the code? ;)

Copy link
Member

Choose a reason for hiding this comment

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

Also merge conflicts need to be resolved.

@dsnet
Copy link
Member

dsnet commented Feb 21, 2018

@andybons, see my comment on #238. Philosophically, I believe it would be inconsistent to close that issue, but accept this one.

@andybons
Copy link
Member

@dsnet fair. Since both fall outside the scope of the tool, closing this one as well.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants