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

Add NoSynopsis option to AddGoComments #67

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Add NoSynopsis option to AddGoComments #67

wants to merge 2 commits into from

Conversation

gmlewis
Copy link
Contributor

@gmlewis gmlewis commented Jan 11, 2023

Fixes: #66.

@samlown samlown added the needs tests Not enough tests for this to be accepted label Jun 12, 2023
@samlown
Copy link
Contributor

samlown commented Jun 12, 2023

Looks good @gmlewis from the outset, but it needs some tests!

@gmlewis
Add unit test for NoSynopsis
1ba1f9c 
Signed-off-by: Glenn Lewis <[email protected]>
@gmlewis
Copy link
Contributor Author

gmlewis commented Jun 27, 2023

Looks good @gmlewis from the outset, but it needs some tests!

Sorry for the delay. I added tests. PTAL.

@glacials glacials mentioned this pull request Jul 16, 2023
Open
samlown
samlown reviewed Sep 6, 2023
View reviewed changes
Copy link
Contributor

@samlown samlown left a comment

Choose a reason for hiding this comment

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

A few people are trying to deal with this, so I appreciate its important. A think a simpler approach without the configuration helpers methods would be best at the moment.

comment_extractor.go
Comment on lines +16 to +18
// ExtractGoCommentsOption represents an option that can be passed to ExtractGoComments
// in order to modify its behavior.
type ExtractGoCommentsOption func(e *extractOptions)
Copy link
Contributor

Choose a reason for hiding this comment

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

I do like this approach for option handling! But I think at the moment it is too inconsistent with the current approach (unfortunately). I have a plan in mind to refactor all configuration towards config methods.

@joanestebanr
Copy link

There are any news about this PR? I'm looking for this feature also

@gmlewis
Copy link
Contributor Author

gmlewis commented Oct 25, 2023

There are any news about this PR? I'm looking for this feature also

If I'm not mistaken, I'm waiting for the following to happen, correct?

I have a plan in mind to refactor all configuration towards config methods.

Or is there something I'm supposed to do to this PR in the meantime?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@samlown samlown samlown left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
needs tests Not enough tests for this to be accepted
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature request: Add ability to preserve comment newlines in AddGoComments
3 participants
@gmlewis @samlown @joanestebanr