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.

Sign up for GitHub

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

Jump to bottom

New lexer: lustre #905

Merged
merged 20 commits into from
Aug 23, 2019
Merged

New lexer: lustre #905

merged 20 commits into from
Aug 23, 2019

Conversation

jahierwan
Copy link
Contributor

No description provided.

@jahierwan
Copy link
Contributor Author

jahierwan commented Apr 26, 2018 •
edited
Loading

I've also put the lutin lexer in this pull request as lustre and lutin are both new and similar

@jahierwan
Copy link
Contributor Author

up ?

vidarh
vidarh suggested changes Jan 11, 2019
View reviewed changes
Copy link
Contributor

@vidarh vidarh left a comment

Choose a reason for hiding this comment

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

Hi, I'm trying to help triage lexer submissions for Rouge, to see if we can get the backlog down. Only thing I can see for yours is that the demos are quite long. Can you please take a look.

lib/rouge/demos/lutin Outdated Show resolved Hide resolved
@vidarh vidarh mentioned this pull request Jan 17, 2019
Closed
@jahierwan
Copy link
Contributor Author

is there something i can do what this ci failure ?

@pyrmont pyrmont mentioned this pull request May 13, 2019
Merged
@pyrmont pyrmont added the needs-review The PR needs to be reviewed label Jul 16, 2019
@pyrmont
Copy link
Contributor

pyrmont commented Aug 2, 2019

@jahierwan I'm sorry it's taken such a long time to get to this submission >_<

I've rebased your branch against the most up to date version of master and force pushed the code to your fork. I can see the logic for why you've combined the Lutin and Lustre lexers but could I trouble you to create a branch off this and pull them apart? They don't share any code and should be submitted separately.

I tried to do it for you but unfortunately I only have commit rights to the branch you've used to submit the PR; I can't create a new branch on your fork.

pyrmont
pyrmont suggested changes Aug 2, 2019
View reviewed changes
Copy link
Contributor

@pyrmont pyrmont left a comment

Choose a reason for hiding this comment

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

Let me know if you have any questions!

lib/rouge/demos/lustre Outdated Show resolved Hide resolved
lib/rouge/lexers/lustre.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/lustre.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/lustre.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/lustre.rb Outdated Show resolved Hide resolved
lib/rouge/lexers/lutin.rb Outdated Show resolved Hide resolved
spec/visual/samples/lustre Outdated Show resolved Hide resolved
spec/visual/samples/lustre Outdated Show resolved Hide resolved
@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels Aug 2, 2019
@pyrmont pyrmont added needs-review The PR needs to be reviewed and removed author-action The PR has been reviewed but action by the author is needed labels Aug 19, 2019
pyrmont
pyrmont reviewed Aug 19, 2019
View reviewed changes
lib/rouge/lexers/lustre.rb Outdated Show resolved Hide resolved
@pyrmont pyrmont added author-action The PR has been reviewed but action by the author is needed and removed needs-review The PR needs to be reviewed labels Aug 19, 2019
@jahierwan jahierwan mentioned this pull request Aug 20, 2019
Merged
@pyrmont pyrmont added needs-review The PR needs to be reviewed author-action The PR has been reviewed but action by the author is needed and removed author-action The PR has been reviewed but action by the author is needed needs-review The PR needs to be reviewed labels Aug 21, 2019
@pyrmont pyrmont merged commit e84edc4 into rouge-ruby:master Aug 23, 2019
@pyrmont pyrmont removed the author-action The PR has been reviewed but action by the author is needed label Aug 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pyrmont pyrmont pyrmont requested changes

@vidarh vidarh vidarh requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jahierwan @pyrmont @vidarh