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 rake task to check grammars #4451

Merged
merged 6 commits into from
Apr 10, 2019

Conversation

lildude
Copy link
Member

@lildude lildude commented Mar 7, 2019

Description

In my ongoing quest to prevent losing a grammar again without noticing, this PR adds a new rake task that is run whenever the grammars are updated. It should confirm that no grammars have suddenly gone missing and also flag if there are any new errors.

Sadly, we have to rely on a hardcoded known error account until such time as we've got 100% error free grammars, which isn't likely to happen any time soon. This is however sufficient enough to prompt anyone making a new release to check the output before actually releasing.

While I was updating the release section on the CONTRIBUTING.md, I also tweaked the dependencies and getting started sections following the feedback in #4449

Checklist:

  • I am adding new or changing current functionality
    • I have added or updated the tests for the new or changed functionality.

For example, on macOS with [Homebrew](http://brew.sh/): `brew install cmake pkg-config icu4c && brew cask install docker` and on Ubuntu: `apt-get install cmake pkg-config libicu-dev docker-ce`.

The latest version of Bundler v1 can be installed with `gem install bundler -v "~>1.10"`.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've moved the dependencies above "Getting started" as it's a lot easier to get started once you've met all the dependencies 😁

@lildude lildude mentioned this pull request Mar 12, 2019
@davidhq
Copy link
Contributor

davidhq commented Mar 12, 2019

Perfect :) Since then I sent you a pull request for a preliminary version of Syntax Highlighter for Vyper (second Ethereum smart contract language), used for example in these contracts: https://github.com/Uniswap/contracts-vyper/blob/master/contracts/uniswap_exchange.vy

I think it's a low risk to include this in current release, so if you manage... if not, would be great if next time. Highlighter follows the same agreement as the one for Solidity (.sublime-syntax on master and tmLanguage on linguist branch)... both will further improve in next weeks to nail down the smaller details.

@lildude lildude requested a review from pchaigno April 4, 2019 12:08
@lildude lildude requested a review from Alhadis April 8, 2019 09:58
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.

Nothing glaring that jumps out at me, but don't forget I have a very bastardised knowledge of Ruby which is ultimately product of Perl and CoffeeScript knowledge. 😉 (Both languages are similar enough to Ruby that every assumption I've ever made about the language has so far been correct)/.

Rakefile Outdated Show resolved Hide resolved
Rakefile Outdated Show resolved Hide resolved
@Alhadis
Copy link
Collaborator

Alhadis commented Apr 8, 2019

Eh... that review was supposed to have been a comment, not a change request. Bit gauche of me, sorry. 😞

@lildude
Copy link
Member Author

lildude commented Apr 8, 2019

Eh... that review was supposed to have been a comment, not a change request. Bit gauche of me, sorry. 😞

NP. All the same to me 😄

@lildude lildude merged commit 7551e73 into master Apr 10, 2019
@lildude lildude deleted the lildude/update-contrib-and-grammar-compile-checks branch April 10, 2019 13:10
@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.

3 participants