Skip to content

Conversation

@chrissimpkins
Copy link
Member

@chrissimpkins chrissimpkins commented Aug 22, 2017

@burodepeper

This commit is based on contributing-docs and adds a draft of issue reporting and pull request documentation to the CONTRIBUTING.md file.

Let me know what you think.

TODO:

- screenshot images that visually demonstrate the problem

Please describe what led to the problem in detail.

Copy link
Member

Choose a reason for hiding this comment

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

The following isn't required, but helps us tremendously:

  • An inline copy of the source code involved, especially if non-ASCII characters are involved.
  • The steps required to replicate the issue, if it involves more than simply looking at your code in an editor.
  • The things you've tried yourself to fix or work around the issue.
  • Any ideas you have an what a good solution would be.

CONTRIBUTING.md Outdated

Issue reports from users are extremely important to foster the ongoing development of the typeface. If you identify a problem, we request that you report it through a new issue report on the Github repository. Please include the following information in your (bug) issue report:

- Hack font version
Copy link
Member

Choose a reason for hiding this comment

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

I tried looking this up for the version I have installed on my macOS machine, and I have no (quick) idea where to find this, besides from opening the file in Glyphs and looking at the metadata. Since releases are quite far apart, I think a rough estimate of when the font was installed is a good enough indication to deduce the version, and possibly easier for the user. So perhaps: "Your version of Hack, or a rough estimate of when you've installed our wonderful font"

Copy link
Member Author

Choose a reason for hiding this comment

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

This sounds good. We can help them troubleshoot this in a platform dependent way. On OS X, you can find the version by clicking the font name in FontBook. It's listed under the version string of the metadata located there. This will vary in a platform dependent way. Hopefully they will research how to do this before they submit a report but I am not opposed to providing assistance to get this information. The fonts are out in the wild in a large variety of versions (e.g. if they install from Font Squirrel which does not track our releases, they will have a different version than a release that took place within a specific timeframe). Not a big issue.

Copy link
Member

Choose a reason for hiding this comment

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

On OS X, you can find the version by clicking the font name in FontBook.

But only after you've found and changed the View settings. It took me longer than I expected to find it. I suggest we add a link to a F.A.Q. that has some more detailed instructions.

Copy link
Member Author

Choose a reason for hiding this comment

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



# Issue Reporting

Copy link
Member

Choose a reason for hiding this comment

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

Before you submit a new issue, make sure you've installed the latest version of Hack. (+include link)

Copy link
Member Author

Choose a reason for hiding this comment

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

Good idea. This may head off problems before they are reported as such!

CONTRIBUTING.md Outdated

# Pull Requests

We highly encourage contributions to the Hack typeface source code, repository scripts, and documentation. Please read and understand our design philosophy statement above in order to avoid frustration with work that we cannot merge upstream. We are willing to consider pull requests that follow these design guidelines. If you intend to submit changes that fall outside of the design guidelines, we highly suggest that you post an issue report with the proposed changes for discussion *before you do the work*. There is never wasted work. If a change is of value to you, it is likely to be of value to others and this is the perfect situation for a downstream fork of Hack that you can maintain and share with other users.
Copy link
Member

Choose a reason for hiding this comment

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

I think there should be more emphasis on the "before you do the work" part. Any PR that involves an amount of work of any significance would benefit from some discussion beforehand. That would also allow us to provide some structure on how to integrate these changes as smoothly as possible.

Copy link
Member Author

Choose a reason for hiding this comment

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

If the intent is to push upstream. In some cases, users may be more than willing to manage a new downstream fork and pushing to let us know that the changes exist without an assumption that they are acceptable. I would really like to encourage that for many of the "I like this better than what you did" suggestions that we have seen in issue reports. As for integration issues, I completely agree. I wanted to see what Glyphs did to the UFO as an example of how a PR might come in to us. I suggest that we add files that can be modified and cannot be modified to the contributing documentation for pull requests. It would be ideal to avoid the need to review features.fea changes and fontinfo.plist changes that are unintentional from contributors as part of a pull request. For optional files that do not influence builds (e.g. the lib.plist changes here) we can be more flexible. I propose that the contributor has to eliminate all of those unintentional changes in order for the PR to be considered, even if that means work occurring outside of their cloned repo and then file copies into the repo for changes that are of import to the modification of the design (without those that influence the metadata). This offloads unnecessary work on us during design reviews. There will be plenty of work to do from a design review standpoint. If you have other thoughts about design specific guidelines, feel free to add those to the document. I think the more explicit that we are with guidance the better.

Copy link
Member

Choose a reason for hiding this comment

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

My two cents on design reviews is that it's a strongly opinionated area, and as such, the conversation about the design is more important than the finished design. There's no one size fits all approach, so I would prefer to initially focus on the discussion as the key component for accepting a PR.

Copy link
Member Author

Choose a reason for hiding this comment

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

My two cents on design reviews is that it's a strongly opinionated area, and as such, the conversation about the design is more important than the finished design. There's no one size fits all approach, so I would prefer to initially focus on the discussion as the key component for accepting a PR.

Mind adding this language to the document so that I can see what you have in mind?

Copy link
Member

Choose a reason for hiding this comment

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

Added it to your PR

Copy link
Member Author

Choose a reason for hiding this comment

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

Perfect! Will pull these changes and review the entire document tonight. Will modify with the TODO list information and then let you know when those changes are available for review.

# Pull Requests

We highly encourage contributions to the Hack typeface source code, repository scripts, and documentation. Please read and understand our design philosophy statement above in order to avoid frustration with work that we cannot merge upstream. We are willing to consider pull requests that follow these design guidelines. If you intend to submit changes that fall outside of the design guidelines, we highly suggest that you post an issue report with the proposed changes for discussion *before you do the work*. There is never wasted work. If a change is of value to you, it is likely to be of value to others and this is the perfect situation for a downstream fork of Hack that you can maintain and share with other users.

Copy link
Member

Choose a reason for hiding this comment

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

How about we add a short list of things users can contribute to? We can tag some of the open issues as applicable, and link to the search query for those. We have some tools that could use an extra set of hands to further develop them. Etc. Would probably make it easier for someone to jump in.

Copy link
Member Author

@chrissimpkins chrissimpkins Aug 22, 2017

Choose a reason for hiding this comment

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

Yeah, I really like this idea. My only hesitation is that it is one more thing to maintain.

Copy link
Member Author

@chrissimpkins chrissimpkins Aug 22, 2017

Choose a reason for hiding this comment

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

Actually, we could just create a new issue report tag for user contributions requested and use a URL that filters based on that? Like that idea?

This would eliminate the need to maintain and push new commits to the documentation. Any of us with repo write access can tag issue reports with this when/if it is something that applies.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that was my idea with "We can tag some of the open issues as applicable, and link to the search query for those." ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 ;)

Copy link
Member Author

Choose a reason for hiding this comment

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

any suggestion about an appropriate label string? "User Contributions Invited"?

Copy link
Member

Choose a reason for hiding this comment

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

"Contribute here" ?

Copy link
Member Author

Choose a reason for hiding this comment

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

or maybe simplify it just to "Contribute"

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, that should work. Or could. Or might. :)

@chrissimpkins chrissimpkins mentioned this pull request Aug 22, 2017
@chrissimpkins
Copy link
Member Author

I will add the checklist modifications as we discussed here and push this. If you don't mind taking a stab at the documentation that you feel is appropriate for PR's based upon preliminary discussions with authors in the form of proposals, we can create a new PR to discuss this further.

Copy link
Member Author

@chrissimpkins chrissimpkins left a comment

Choose a reason for hiding this comment

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

This looks good to me. Reading the diff at the moment. Will take a look in the context of the entire document this evening.

…peper in #275, new pull request file modifications/change docs, new pull request for script mods docs, new pull request for documentation docs, new "who is a contributor" docs
@chrissimpkins
Copy link
Member Author

chrissimpkins commented Aug 23, 2017

6b86fce includes all modifications that we discussed and other changes in the OP TODO list. Also added a short section that describes our definition of a "contributor". Please have a look and confirm that you agree with this philosophy. It is the approach that we have adopted in the past.

@chrissimpkins
Copy link
Member Author

Most recent set of revision recommendations are available in 45a2adb.

Let me know if you want to modify or add anything else, whether this is ready for merge to master branch.

@burodepeper
Copy link
Member

We can always add/modify later on. I'm going to merge it :)

@burodepeper burodepeper merged commit 03adf14 into contributing-docs Aug 23, 2017
@chrissimpkins
Copy link
Member Author

👍

@chrissimpkins
Copy link
Member Author

Will prune the contributing-docs-IRPR branch. OK with merge to master now? We are currently merged to contributing-docs branch. Should be clean merge to master

@chrissimpkins chrissimpkins deleted the contributing-docs-IRPR branch August 23, 2017 13:51
@burodepeper
Copy link
Member

Sure!

@chrissimpkins
Copy link
Member Author

Done! Currently in master. Will prune contributing-docs branch as no longer necessary and all commits in master from this branch. Looks good!

CodingMarkus pushed a commit to CodingMarkus/DockerHackFont that referenced this pull request Oct 9, 2018
…peper in source-foundry#275, new pull request file modifications/change docs, new pull request for script mods docs, new pull request for documentation docs, new "who is a contributor" docs
CodingMarkus pushed a commit to CodingMarkus/DockerHackFont that referenced this pull request Oct 9, 2018
…g-docs-IRPR

adds issue reporting and pull request documentation to contributing docs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants