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

Don't count VCL as Perl for statistics. #3857

Merged
merged 2 commits into from
Oct 12, 2017

Conversation

dcmoore-gd
Copy link
Contributor

@dcmoore-gd dcmoore-gd commented Oct 11, 2017

While the Varnish-specific language was apparently inspired by C and Perl, there's no reason to group it as Perl for repo statistics. Also re-adding the VCL color definition.

While the Varnish-specific language was apparently inspired by C and Perl, there's no reason to group it as Perl for repo statistics.
@dcmoore-gd
Copy link
Contributor Author

I could also see setting the group as "Other" or "Vendor-Specific"/"Domain-Specific" if these kinds of languages should be grouped together, but that doesn't seem to be a pattern in languages.yml at this time.

Copy link
Collaborator

@Alhadis Alhadis left a comment

Choose a reason for hiding this comment

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

Agreed. From the language's documentation, the relation to Perl is superficial at best. It's definitely not Perl syntax, either.

@pchaigno
Copy link
Contributor

@gjtorikian added the Perl group to VCL in #2298. Could be an oversight or he had some legitimate reason. @gjtorikian Do you remember?

@gjtorikian
Copy link
Contributor

Oh, it was almost certainly an oversight. In fact, I’m a bit embarrassed to see that I deleted the color, too. 😨 It might be nice to define that back in here again.

@dcmoore-gd
Copy link
Contributor Author

I can make that color change too as part of this PR.

@Alhadis
Copy link
Collaborator

Alhadis commented Oct 12, 2017

In fact, I’m a bit embarrassed to see that I deleted the color, too

The colour deletion was justified if VCL was grouped under Perl, because it would never have had a chance to be used or seen in the language bar. Nothing to feel embarrassed about. =)

@dcmoore-gd
Copy link
Contributor Author

Thanks @Alhadis and @gjtorikian for your work on this project and feedback on this PR! VCL's color has been restored in this PR now too and CI tests, they are a'passin.

@gjtorikian
Copy link
Contributor

Thanks!

@gjtorikian gjtorikian merged commit a0b38e8 into github-linguist:master Oct 12, 2017
@lildude lildude mentioned this pull request Oct 14, 2017
@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