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 commenter #25

Merged
merged 2 commits into from Jan 20, 2021
Merged

Add commenter #25

merged 2 commits into from Jan 20, 2021

Conversation

Saberos
Copy link
Contributor

@Saberos Saberos commented Jan 18, 2021

Recently discovered this plugin. Loved it, but missed the commenting shortcut.

First time contributing to a project and first time working on an IntelliJ plugin, so any suggestions are welcome.

Used documentation:
Commenter
Commenter testing

@Mic92
Copy link
Member

Mic92 commented Jan 19, 2021

@JojOatXGME for review.

@JojOatXGME JojOatXGME self-requested a review January 19, 2021 18:12
@JojOatXGME
Copy link
Contributor

JojOatXGME commented Jan 19, 2021

Great contribution. You even created the first Java test within the repository. 👍 I hope I can add some more tests in the near future. About general suggestions: I can only think of one little extra. If your solution is based on some reference on the web, it might be a nice extra for reviewers if you link the reference in the pull request.

Copy link
Contributor

@JojOatXGME JojOatXGME left a comment

Choose a reason for hiding this comment

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

I noticed that your commit seems to contain an email address which is not recognized by GitHub. Are you aware of that? This is not necessarily a problem, I just want to confirm that you did not use the wrong email address by mistake.
(EDIT: Here you can find some more information by GitHub)

It would be nice if you can bump the version number and add your changes to the changelog. You can change the version number in gradle.properties. For the changelog, you can run the Gradle task :patchChangelog before you make your changes on the CHANGELOG.md. See 22bf94c as an example. Otherwise, I will do it later this week.

@Saberos
Copy link
Contributor Author

Saberos commented Jan 20, 2021

@JojOatXGME did not think of configuring the correct git user information for GitHub. Thanks for the heads up, I've corrected it.

I've added the change to the changelog and bumped the version, is this what you had in mind?

And although you already approved I added some links I used to the PR for completeness.

@JojOatXGME JojOatXGME merged commit 32cd831 into NixOS:master Jan 20, 2021
@JojOatXGME
Copy link
Contributor

Yes, that is what I meant. I have merged your changes. Thanks again for your contribution.

@JojOatXGME
Copy link
Contributor

Since this pull request was already merged a few weeks ago, and I haven't finished my work on the parser yet, I think I would build a release just for the commenter. @Mic92 Any objections?

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