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

Move Starlark to separate language #4759

Merged
merged 10 commits into from
Jan 23, 2020
Merged

Move Starlark to separate language #4759

merged 10 commits into from
Jan 23, 2020

Conversation

arturdryomov
Copy link
Contributor

@arturdryomov arturdryomov commented Dec 27, 2019

Starlark is a scripting language inspired by Python.

Starlark is mostly a Bazel build system child. But it gains a bit of traction. For example, the Buck build system adopts it as well. Interestingly Drone CI supports Starlark as a configuration language.

GitHub stats:

What this change does:

  • defines a new Starlark language based on Python semantics;
  • moves everything Starlark, Bazel (BUILD, WORKSPACE) and Buck (BUCK) related from Python to Starlark;
  • moves samples and adds a new one with the .starlark extension;
  • since Starlark is a part of the Bazel family I’ve defined the color as a Bazel green one (preview).

CC @jin, @artem-zinnatullin

Copy link

@artem-zinnatullin artem-zinnatullin left a comment

Choose a reason for hiding this comment

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

Noice

extensions:
- ".bazel"
- ".bzl"
- ".starlark"
Copy link
Contributor

Choose a reason for hiding this comment

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

Copybara uses *.sky as well.

The Go Starlark interpreter examples uses *.star.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think .sky should be abandoned since Skylark is Starlark at this point. Gonna add .star since there are 20K hits and it might be a widespread extension in the future. Thanks!

Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll need to remove .starlark because it falls short of our usage requirements (only 41 files at the time of writing). Otherwise, this looks file to me.

@@ -390,6 +390,7 @@ This is a list of grammars that Linguist selects to provide syntax highlighting
- **Squirrel:** [textmate/c.tmbundle](https://github.com/textmate/c.tmbundle)
- **Stan:** [jrnold/atom-language-stan](https://github.com/jrnold/atom-language-stan)
- **Standard ML:** [textmate/standard-ml.tmbundle](https://github.com/textmate/standard-ml.tmbundle)
- **Starlark:** [MagicStack/MagicPython](https://github.com/MagicStack/MagicPython)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, typo?

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jin Not a typo. The list is auto-generated from whatever grammars are assigned to a language, which in this case happens to be Python.

extensions:
- ".bazel"
- ".bzl"
- ".starlark"
Copy link
Collaborator

Choose a reason for hiding this comment

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

You'll need to remove .starlark because it falls short of our usage requirements (only 41 files at the time of writing). Otherwise, this looks file to me.

@arturdryomov
Copy link
Contributor Author

@Alhadis, thanks for the review! What are next steps? I guess @lildude does the merging (taking merged PRs into account).

@Alhadis
Copy link
Collaborator

Alhadis commented Dec 29, 2019

Yup, just have to wait for a GitHub staff member to give their 👍 of approval before we can merge.

Copy link
Contributor

@pchaigno pchaigno left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution @arturdryomov!

I've left a couple comments inline. I'm 👍 for the separation of Starlark and Python, but have some concerns regarding the two new file extensions (see below).

/cc @Dominator008 who added the .bzl extension to Python in #2846.

samples/Starlark/sample.star Outdated Show resolved Hide resolved
lib/linguist/languages.yml Show resolved Hide resolved
lib/linguist/languages.yml Show resolved Hide resolved
lib/linguist/languages.yml Outdated Show resolved Hide resolved
lib/linguist/languages.yml Outdated Show resolved Hide resolved
@jin
Copy link
Contributor

jin commented Jan 3, 2020

fyi @laurentlb @alandonovan

@arturdryomov
Copy link
Contributor Author

@pchaigno, do we need to make some changes to merge this? I hope I haven’t missed something 😰

@arturdryomov
Copy link
Contributor Author

@pchaigno, sorry for pinging you again. Is there anything else needs to be changed? Just attempting to not abandon this PR when it slips away from the Recent Activity section on GitHub 😉

@Alhadis
Copy link
Collaborator

Alhadis commented Jan 21, 2020

@arturdryomov We need to wait for a GitHub staff member to give a final 👍 of approval before we can merge. 😉 It's a matter of policy, because Linguist is a production dependency of the site.

@Alhadis Alhadis requested a review from lildude January 21, 2020 01:40
@arturdryomov
Copy link
Contributor Author

@Alhadis, ah, thanks! I thought @pchaigno is from the GitHub team for some reason 🤔 I guess I saw some merges from you guys.

@lildude lildude merged commit ac6d057 into github-linguist:master Jan 23, 2020
@arturdryomov
Copy link
Contributor Author

Huzza! Thanks @lildude!

@arturdryomov arturdryomov deleted the ad/starlark branch January 23, 2020 06:47
adamyi added a commit to adamyi/Geegle3 that referenced this pull request Feb 12, 2020
now that github-linguist/linguist#4759 is in place,
i'm happy to have those counted :)
adamyi added a commit to adamyi/Geegle3 that referenced this pull request Feb 24, 2020
now that github-linguist/linguist#4759 is in place,
i'm happy to have those counted :)
- WORKSPACE
aliases:
- bazel
- bzl
Copy link

@c4milo c4milo Aug 7, 2020

Choose a reason for hiding this comment

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

I see you mentioned .star as one of the extensions in your PR description. Any reason why it was left out?

Copy link
Contributor

Choose a reason for hiding this comment

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

@c4milo Please see #4759 (comment).

@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.

9 participants