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

Fix comments between headers (#447) #448

Merged
merged 1 commit into from
Oct 28, 2019

Conversation

mbodock
Copy link
Contributor

@mbodock mbodock commented Oct 11, 2019

Background

Closes issue #447

Sometimes is nice to provide more information about a specific header, specially when sharing targets files. For this reason would be nice be able to put comments in the header section.

Checklist

  • Git commit messages conform to community standards.
  • Each Git commit represents meaningful milestones or atomic units of work.
  • Changed or added code is covered by appropriate tests.

@mbodock mbodock requested review from tsenart and xla as code owners October 11, 2019 11:57
tsenart
tsenart previously approved these changes Oct 27, 2019
Copy link
Owner

@tsenart tsenart left a comment

Choose a reason for hiding this comment

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

Looks good to me, apart from the in-line suggestion.

lib/targets.go Outdated Show resolved Hide resolved
@mbodock
Copy link
Contributor Author

mbodock commented Oct 27, 2019

@tsenart Applied the suggestion, nice catch!

Also rebased with master in upstream. Please let me know if you need anything else.

@mbodock mbodock requested a review from tsenart October 27, 2019 13:41
tsenart
tsenart previously approved these changes Oct 28, 2019
@tsenart
Copy link
Owner

tsenart commented Oct 28, 2019

One last request: Please update the relevant sections of the README with this added functionality.

Add the possibility to add comment between headers

Fixes tsenart#447
@mbodock
Copy link
Contributor Author

mbodock commented Oct 28, 2019

Just removed the part about targets.

This should be enough since my first usage I already expected to be able to use comments anywhere.

@mbodock mbodock requested a review from tsenart October 28, 2019 10:40
@tsenart tsenart merged commit fe238bd into tsenart:master Oct 28, 2019
@mbodock mbodock deleted the fix-comment-in-headers branch October 28, 2019 12:02
@mbodock
Copy link
Contributor Author

mbodock commented Oct 28, 2019

Hello @tsenart, thank you very much for your time.

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.

2 participants