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 community-focused files for open sourcing #64

Merged
merged 3 commits into from
Jul 2, 2021
Merged

Conversation

kylelaker
Copy link
Contributor

@kylelaker kylelaker commented Jun 26, 2021

So this adds some initial contributing information and updates the README somewhat accordingly but there is still a little bit of work to do here. It also adds a license file, which we may need to break out into a separate PR if this isn't something that we've totally decided on yet.

Usually when I start typing walls of texts I make quite a few typos and I usually don't love re-reading what I've written too many times. I'd greatly appreciate specific recommendations for wording changes or additional commits to this branch (which we can squash/fixup/reorg as needed for cleanliness).

Contributing

As far as the contributing documentation I have made the following assumptions:

  • we want to encourage issues before work
  • we don't yet have a security report process different than our regular bug report process
  • the links I've tried to use work
  • we actually intend to use the (fairly standard) help wanted and good first issue labels

License

I assumed the MIT License. This matches a previous Easy Dynamics project that is available as an open source project and it also matches the documented licenses for at least a few of our dependencies. Again, if we don't want to use MIT, I'd rather remove it from this PR, have that discussion internally, and open a new PR for that rather than hashing it out here; I just wanted a file to link to in the other

Readme

This one I tried to make some changes to but someone more knowledgeable than me on some of the project specifics will need to provide some direct and exact wording that we should use for these TODO sections. The wording is also written with the assumption that #63 gets merged without significant changes to name or structure; it's structure reflects the future state based on that PR. EGRC-422 encompasses more of the changes necessary for recent features and EGRC-425 covers the changes necessary for EGRC-168. I just wanted to get something in that could be referenced from the CONTRIBUTING docs.

What we're missing

Security Reporting Process

Many projects and organizations have a separate process for reporting security vulnerabilities and issues. These tend to be at the organizational level rather than per-project. At this point, I am not entirely convinced that this is necessary for our project. I do think it is a conversation that we should start and drive internally for larger change and guidance.

A Code of Conduct

While internally at Easy Dynamics we have a clear code of conduct and policies, now that we are fostering a community, we should strive to make clear what our expectations are when folks interact with one another. Choosing or writing a code of conduct is important in this. By this point, nearly all major open source projects have adopted codes of conduct. https://www.contributor-covenant.org is popular. AWS has https://aws.github.io/code-of-conduct.

Issue templates

GitHub supports issue templates to make reporting issues a little bit easier. I think that while they can be useful, we may want to take some time to identify the types of interactions we have using the Issues feature before we try to make this overly formal.

Decision on CLA/DCO/neither

I am not sure whether this discussion has been had or not but I am not aware of any decision so I just want to mention it. We will need to decide whether we want to require either a Contributor License Agreement (CLA) or acknowledgement of the Developer Certificate of Origin. Of course, there are many projects that use either or neither.

More resources

For information on the various types of files and configurations that GitHub recommends, see the documentation on Healthy Contributions. Additionally, the GitHub docs repo has an example of a really good CONTRIBUTING.md but I am not sure that we have all the decisions around our process to be that specific yet. Finally, AWS has a repo with their templates for each that some folks on this project may have seen before.

CONTRIBUTING.md Outdated
Comment on lines 44 to 50
[`help wanted` label](/../../issues?q=is%3Aopen+is%3Aissue+label%3A%22help+wanted%22). For issues that
might be especially good for new contributors, we use the
[`good first issue` label](/../../issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22).
Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's not a great way to put comments in markdown files so I am just going to discuss this here. The /../../ is because / links to the root of the branch (so https://github.com/ORG/REPO/tree/BRANCH. So the ../../ then lets you link relative to https://github.com/ORG/REPO without hardcoding. This would allow us to adopt this into an EasyDynamics/.github repository.

@kylelaker kylelaker requested a review from folksgl June 26, 2021 02:07
Copy link
Contributor

@folksgl folksgl left a comment

Choose a reason for hiding this comment

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

Overall, I think this is worded very well. I think MIT will probably be the way we go if that's what EDC has done in the past, and it fits well with the project goals as I understand them.

README.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
README.md Outdated
OSCAL React Library provides components and a sample application for browsing and interacting
with [OSCAL](https://pages.nist.gov/OSCAL) data. The components are reusable in any react
application and the sample application allows browsing OSCAL data, including System Security Plans,
Catalogs, and Components.
Copy link
Contributor

Choose a reason for hiding this comment

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

Profiles now as well.

README.md Outdated Show resolved Hide resolved
README.md Outdated

#### Usage

TODO: Provide an example of using one of the components (or something else that we provide).
Copy link
Contributor

Choose a reason for hiding this comment

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

The Storybook issue will cover this in detail.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
CONTRIBUTING.md Show resolved Hide resolved
CONTRIBUTING.md Outdated

## Making Changes

When contributing a change to the code base, we do ask that you first open a GitHub issue to
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll need probably need people to sign a contributor agreement with something like https://cla-assistant.io/?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was hoping we'd able to avoid that but fair :) Do we have a CLA? Do we need to work internally with the contracts team on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@kylelaker, I don't know that we have an existing CLA, but we will need one. Yes, I'd work with contracts team. Thanks.

CONTRIBUTING.md Outdated Show resolved Hide resolved
README.md Outdated
Comment on lines 12 to 16
![OSCSAL Catalog Viewer Screenshot](docs/resources/catalog-viewer-screenshot.png)
#### Installation

The ssp-example from the OSCAL Github repo is loaded by default in the SSP viewer:
Use the `npm` package manager to install `oscal-react-library` in your project.

![OSCSAL SSP Viewer Screenshot](docs/resources/ssp-viewer-screenshot.png)
Copy link
Contributor

Choose a reason for hiding this comment

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

I noticed the screenshots were removed in the README. I might have missed it, but did we decide to remove these? If so, we could remove the docs/resources folder containing only the unused screenshots.

@kylelaker
Copy link
Contributor Author

I noticed the screenshots were removed in the README. I might have missed it, but did we decide to remove these? If so, we could remove the docs/resources folder containing only the unused screenshots.

Great point. So I largely wrote this with the assumption that the PR splitting this into library + example would be completed. Mostly so that we didn't have to rewrite it again. And this README is in the root of the project. Therefore, it should largely be documenting the library. I don't think that screenshots of the sample application are particularly interesting there. I do think that providing examples of specific components and how to use the library is interesting. But I don't know enough on how to do that.

From the initial PR body, I need help with those specifics and I'd welcome any such help :)

@Bronstrom
Copy link
Contributor

@kylelaker Gotcha, that makes sense - I would definitely be willing to help out on giving examples of specific components and libraries too (I personally don't know about all of it either at this point). Would it be better to have a separate README in example/ to document OSCAL Viewer?

Copy link
Contributor

@tuckerzp tuckerzp left a comment

Choose a reason for hiding this comment

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

With @Bronstrom working on README in #63, everything else looks good to me!

@kylelaker
Copy link
Contributor Author

Any thoughts on the CoC or security reporting parts of the opening message?

@rgauss
Copy link
Contributor

rgauss commented Jul 1, 2021

Any thoughts on the CoC or security reporting parts of the opening message?

Agree on not needing a security reporting process at this time.

I've always liked the Contributor Covenant, let's use that. Any issues with the latest version (2.0)?

@kylelaker
Copy link
Contributor Author

Any thoughts on the CoC or security reporting parts of the opening message?

Agree on not needing a security reporting process at this time.

I've always liked the Contributor Covenant, let's use that. Any issues with the latest version (2.0)?

I like it too! All we need to do is establish a reporting contact under the "Enforcing" heading. Should we use the Innovation Team email address? Is that too open?

@rgauss
Copy link
Contributor

rgauss commented Jul 1, 2021

I like it too! All we need to do is establish a reporting contact under the "Enforcing" heading. Should we use the Innovation Team email address? Is that too open?

DId that get created? If so, sure.

CONTRIBUTING.md Outdated Show resolved Hide resolved
@kylelaker
Copy link
Contributor Author

I like it too! All we need to do is establish a reporting contact under the "Enforcing" heading. Should we use the Innovation Team email address? Is that too open?

DId that get created? If so, sure.

It exists in the GAL! It's in the current version of the CODE_OF_CONDUCT.md.

@kylelaker
Copy link
Contributor Author

Thoughts on removing README.md from this PR and putting it on its own branch; that way this will no longer be blocked pending #63?

@rgauss
Copy link
Contributor

rgauss commented Jul 1, 2021

Thoughts on removing README.md from this PR and putting it on its own branch; that way this will no longer be blocked pending #63?

Sure.

@kylelaker kylelaker force-pushed the EGRC-419 branch 2 times, most recently from 2dadd0f to de23817 Compare July 2, 2021 00:09
@kylelaker
Copy link
Contributor Author

README changes have been "stashed" on EGRC-419-README. This branch has been rebased/force-pushed/modified to have the README removed from all commits (which therefore resulted in a commit being removed). That way it won't be possible to conflict with other branches modifying the README.

The MIT license is being added to the repository. This fits with the
ecosystem we are part of, is compatible with our dependencies, and
allows permissive use of the code in the repository.
We need basic documentation on how to contribute in order to facilitate
a community around this project. This is early documentation which is
likely to grow and improve quite a bit going forward. Initially, we want
to ask that contributors create issues prior to PRs and give some
general guidance on what the process will look like after a PR has been
opened.
This ensures that our project has a code of conduct, which we can use to
help ensure that the community surrounding this project is healthy and
welcoming.
@rgauss
Copy link
Contributor

rgauss commented Jul 2, 2021

@kylelaker, anything preventing this from being merged at this point?

@kylelaker
Copy link
Contributor Author

Not from me. At this point, if could review my own PR, I'd give it the green check :)

To summarize everything so far:

  • CONTRIBUTING docs added
  • COC is Contributor Covenant (reporting contact is innovation@)
  • Separate security report is unnecessary (at this time)
  • README is deferred until after Build OSCAL React library #63
  • CLA is deferred until at least next week
  • Issue templates deferred until we actually get some from the community
  • License is MIT for reasons outlined in the PR body and commit message

@rgauss rgauss requested a review from folksgl July 2, 2021 15:43
@rgauss
Copy link
Contributor

rgauss commented Jul 2, 2021

@kylelaker, I re-requested a review from @folksgl, but I think his comments were addressed, so feel free to merge if you'd like.

@kylelaker kylelaker merged commit 055c7f9 into develop Jul 2, 2021
@kylelaker kylelaker deleted the EGRC-419 branch July 2, 2021 15:49
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

5 participants