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

Event #49

Merged
merged 1 commit into from
Apr 4, 2019
Merged

Event #49

merged 1 commit into from
Apr 4, 2019
+647 −505

Conversation

brettz9
Copy link
Contributor

@brettz9 brettz9 commented Nov 2, 2018

Builds on PR #48.

@Kuniwak Kuniwak self-requested a review November 2, 2018 11:29
Kuniwak
Kuniwak approved these changes Nov 2, 2018
View reviewed changes
peg_src/jsdoctype.pegjs Outdated Show resolved Hide resolved
Kuniwak
Kuniwak requested changes Nov 2, 2018
View reviewed changes
Copy link
Member

@Kuniwak Kuniwak left a comment

Choose a reason for hiding this comment

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

Oops, I have approved accidentally...

@brettz9 brettz9 force-pushed the event branch 3 times, most recently from 033fcd2 to 554b051 Compare November 3, 2018 14:19
@brettz9
Copy link
Contributor Author

brettz9 commented Nov 12, 2018

Would you be able to take another look? I think this should be ready now...

@brettz9
Copy link
Contributor Author

brettz9 commented Dec 7, 2018

Could I ask for an ETA on when you might be able to take a look at this?

@brettz9
Copy link
Contributor Author

brettz9 commented Mar 28, 2019

Bump...

@brettz9 brettz9 mentioned this pull request Mar 28, 2019
Open
sandersn
sandersn approved these changes Apr 3, 2019
View reviewed changes
Copy link
Contributor

@sandersn sandersn left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I think this addresses @Kuniwak's concerns now.

@brettz9
- Fix: Allow and report event: prefix within ModulePathExpr and `…
94483f2 
…NamepathExpr`; defined at http://usejsdoc.org/tags-event.html#syntax and used within module path at http://usejsdoc.org/tags-listens.html#example (fixes jsdoctypeparser#47)

- Testing: Add  test for member (and event member)
@brettz9
Copy link
Contributor Author

brettz9 commented Apr 4, 2019

I've rebased, but it seems I am not able to push my commit to master in jsdoctypeparser, despite seeing myself listed on the project (I guess I need to be enabled for the repo specifically?).

I'm also happy to follow a procedure of submitting PRs if that is desired, but with changes requested here by Kuniwak (and with this project creator no longer active), should I submit a new PR?

@sandersn
Copy link
Contributor

sandersn commented Apr 4, 2019 •
edited
Loading

Hm, I thought org members would be able to commit freely. I made you an org owner instead of just adding you to the contributor list for the repo. I don't think there's much social difference. Maybe some technical difference.

I'd like to ship 3.1.0 today [edit: UTC -7] so I'll go ahead and merge this PR along with your others.

@sandersn sandersn merged commit de24f93 into jsdoctypeparser:master Apr 4, 2019
@brettz9 brettz9 deleted the event branch April 4, 2019 16:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Kuniwak Kuniwak Kuniwak requested changes

@sandersn sandersn sandersn approved these changes

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

Successfully merging this pull request may close these issues.

Allow "event:" within types as per JSDoc specs
3 participants
@brettz9 @sandersn @Kuniwak