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

Add Apache thrift support to generated? check #2392

Merged
merged 4 commits into from
May 20, 2015

Conversation

vighnesh1987
Copy link
Contributor

Apache Thrift compiler generated files are currently ignored in the generated? check leading to massive PRs which are hard to parse. This commit fixes the same.

@zfogg
Copy link

zfogg commented May 12, 2015

+1

@pchaigno
Copy link
Contributor

Could this be limited to only files with a certain extension like we did in the other methods?

@vighnesh1987
Copy link
Contributor Author

@pchaigno Good idea, added

return false unless ['.rb', '.py', '.go', '.js', '.m', '.java', '.h', '.cc', '.cpp'].include?(extname)
return false unless lines.count > 1

return lines[0].include?("Autogenerated by Thrift Compiler") || lines[1].include?("Autogenerated by Thrift Compiler")
Copy link
Contributor

Choose a reason for hiding this comment

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

@arfon @bkeepers Should we change this to lines[0..1].join("").include?("Autogenerated by Thrift Compiler")?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure which would be faster. I find the the current implementation more readable.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, let's keep the current one (I was kind of hoping for a better way to write it in Ruby...).

@arfon
Copy link
Contributor

arfon commented May 20, 2015

👍 thanks for this @vighnesh1987. This should be live on GitHub with the next release of Linguist.

arfon added a commit that referenced this pull request May 20, 2015
Add Apache thrift support to generated? check
@arfon arfon merged commit 5ff0d48 into github-linguist:master May 20, 2015
@vighnesh1987
Copy link
Contributor Author

Awesome!

@vighnesh1987 vighnesh1987 deleted the apache-thrift branch May 20, 2015 18:37
@arfon arfon mentioned this pull request Jun 17, 2015
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants