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 support for Cloud Firestore Security Rules #4120

Merged
merged 4 commits into from May 1, 2018
Merged

Add support for Cloud Firestore Security Rules #4120

merged 4 commits into from May 1, 2018

Conversation

Alhadis
Copy link
Collaborator

@Alhadis Alhadis commented Apr 30, 2018

This PR adds Cloud Firestore Security Rules (hereafter just "Firestore Rules") to the list of recognised languages on GitHub.com.

Description

A user unknowingly brought the language to my attention when he asked about adding Firebase's icon to firebase.rules files. Did some investigation, and found ~2,091 search results for files named firebase.rules. I'm not sure if there are other names we should concern ourselves with. The contributor who brought this format to my attention said:

afaik it could be any *.rules file, but the de facto standard is firestore.rules (at least for firestore). I haven't seen *.rule yet

Checklist:

@Alhadis
Copy link
Collaborator Author

Alhadis commented Apr 30, 2018

Also, think I solved that issue we've been having:

λ linguist (cfssr): script/add-grammar https://github.com/jaysquared/atom-firestore-grammar
Checking docker is installed and running
$ docker ps
Registering new submodule: vendor/grammars/atom-firestore-grammar
$ git submodule add -f https://github.com/jaysquared/atom-firestore-grammar vendor/grammars/atom-firestore-grammar
$ script/grammar-compiler add vendor/grammars/atom-firestore-grammar
Confirming license
$ script/licensed --module /home/alhadis/Mirrors/linguist/vendor/grammars/atom-firestore-grammar
Updating grammar documentation in vendor/README.md
$ bundle exec rake samples
$ script/sort-submodules
$ script/list-grammars
  > script/list-grammars:81:in `block in to_markdown': undefined method `chomp' for nil:NilClass (NoMethodError)
  > 	from script/list-grammars:65:in `each'
  > 	from script/list-grammars:65:in `to_markdown'
  > 	from script/list-grammars:96:in `update_readme'
  > 	from script/list-grammars:105:in `<main>'
Command failed. Aborting.
λ linguist (cfssr): 

Since adding this, it hasn't had a single complaint about chomping anything I tell it to eat. =)

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.

My initial reaction: What?! @Alhadis didn't write the grammar for that language? Oh wait, he did post a pull request with a full rewrite of the grammar...

@Alhadis
Copy link
Collaborator Author

Alhadis commented May 1, 2018

😀Haha! Yeah, now we just need to wait for the maintainer to approve it. 😉

Going by his recent activity, I'm wondering if I shouldn't try bugging him by e-mail... there's another user's PR that's waiting to be merged too...

@Alhadis
Copy link
Collaborator Author

Alhadis commented May 1, 2018

Oh, and about this...

  ace_mode: less
  codemirror_mode: css
  codemirror_mime_type: text/css

The highlighting applied by CodeMirror and Ace's CSS modes looks weirdly appropriate when applied to Firestore Rules, which is why I chose that over simply defaulting to plain-text. =)

Copy link
Member

@lildude lildude left a comment

Choose a reason for hiding this comment

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

LGTM

@Alhadis Alhadis merged commit d8e4680 into master May 1, 2018
@Alhadis Alhadis deleted the cfssr branch May 1, 2018 08:05
@pchaigno
Copy link
Contributor

pchaigno commented May 1, 2018

Going by his recent activity, I'm wondering if I shouldn't try bugging him by e-mail... there's another user's PR that's waiting to be merged too...

I'd try pinging him via GitHub.com (i.e., @hisusername) in a few days if he hasn't answered. That worked well for me in the past; I think many users receive an email notification when they're pinged. I try to keep cold-emails as a last resort.

@Alhadis
Copy link
Collaborator Author

Alhadis commented May 1, 2018

Just so we're all clear, if you ever see me silent for over a month, it legitimately means I'm dead.

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

Successfully merging this pull request may close these issues.

None yet

3 participants