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

NASA-AMMOS SLIM Repository Essentials Assistance -- Baseline Files #442

Closed
wants to merge 2 commits into from

Conversation

jpl-jengelke
Copy link

@jpl-jengelke jpl-jengelke commented Jan 8, 2024

Purpose

  • Infusion of documentation and ticket/PR templating suggestions for SLIM integration

Proposed Changes

Automation templates for the following:

  • Pull Requests
  • Issue Tickets
  • Feature Request Tickets

MGSS-approved and customized (application-specific) documentation templates:

  • CHANGELOG.md
  • CODE_OF_CONDUCT.md
  • CONTRIBUTING.md
  • LICENSE
  • images in support of documentation in webdocs directory

Leveraging SLIM Best Practice Guides:
https://nasa-ammos.github.io/slim/docs/category/documentation
https://nasa-ammos.github.io/slim/docs/category/contributions

- ⚠ NOTE: This PR branch should be customized / adapted - please commit changes before merging!
- ⚠ NOTE: Modify by searching text ('INSERT' keyword) and/or replacing with appropriate values.
- ⚠ NOTE: Read the file thoroughly to ensure it's well adapted and please freely modify as necessary.

Issues

Testing

  • Reviewed by developer and verified by Git Diff tool
  • Documentation additions, no testing available

@gkjohnson
Copy link
Contributor

gkjohnson commented Jan 9, 2024

Hi @jpl-jengelke - can you provide some more context for these changes? At lot of them are redundant and overwrite existing decisions and tracking from the project (ie the CHANGELOG, issue templates, LICENSE copy). Why are they necessary?

Also a lot of your links don't work (links to the SLIM issue and SLIM web pages).

For a bit more information I'm responsible for the following projects in the NASA-AMMOS org:

  • 3DTilesRendererJS
  • CameraUtilsJS
  • 3DTilesSampleData
  • TimecraftJS (though I don't currently have edit access to the project I did write most of the code)

@jpl-jengelke
Copy link
Author

Hi @jpl-jengelke - can you provide some more context for these changes? At lot of them are redundant and overwrite existing decisions and tracking from the project (ie the CHANGELOG, issue templates, LICENSE copy). Why are they necessary?

Also a lot of your links don't work (links to the SLIM issue and SLIM web pages).

For a bit more information I'm responsible for the following projects in the NASA-AMMOS org:

  • 3DTilesRendererJS
  • CameraUtilsJS
  • 3DTilesSampleData
  • TimecraftJS (though I don't currently have edit access to the project I did write most of the code)

Hi Garrett,

Thank you for reading through our PR. I'm on a team with @riverma who are working on Lab standards for software, and we also manage this GitHub project (project, not repo) in public GitHub. We are working on presenting unified documentation for all repos in the project, and that is what this PR is attempting to infuse. We hope that the suggested changes can be embraced with/without some modifications to make the standards relevant to this particular software repo.

In the instance of some of the repos above, it appears that the suggestions from the SLIM project have already been embraced in some form. In others, the templates would be new additions to the repositories to create the suggested documentation stack. For this repo, in particular, it looks like there are a few files that would contribute. Others, however, are already implemented and so are sufficiently standardized that they meet Lab objectives.

Specifically:

  • CHANGELOG is per standard, perhaps it was introduced previously from SLIM?
  • Issue templates do not appear to differ substantively from SLIM, but it would be appreciated if the merge request would be analyzed to determine if the existing templates can be enhanced (or already) provide requested information.
  • LICENSE changes have been reverted pending further guidance from this ticket.

Please note that there is not a suggestion to accept files without granularity, please examine them to prevent overwrite with no-op information and accept or integrate only that which will improve the documentation stack: `NOTE: This PR branch should be customized. ... Freely modify as necessary."

The two links reported have been repaired, so thank you for the heads up.

Best Regards,

John Engelke

@gkjohnson
Copy link
Contributor

gkjohnson commented Jan 10, 2024

Great - thanks for info. I can look through the remaining files and take what's needed but generally the state of the repo is based on what I've found works for my active open source projects after working on them for 7 or 8 years.

I appreciate and am glad to see an effort to provide consistency across projects but If you're open to feedback I think a lot of what's included here is overly verbose and extremely bureaucratic looking. For the types of projects NASA AMMOS open sources I think it's best to make it as easy as possible to contribute and not turn potential users and contributors away.

The "CONTRIBUTING.md" document in particular is extremely long and not something I would ever read before contributing to a project. It should be short and answer necessary questions people have before contributing to a project but not a lot more. Currently it includes a lot of basic Git explanations and complex rules that I've never seen properly followed in any project I've worked on (commit message requirements, sign offs, etc). They're nice in theory but it don't work in practice. In the worst case these kinds of documents and wording can scare people away from contributing to a project - which I've seen happen with NASA AMMOS projects in the past.

I'm happy to talk about these things more if you'd like.

CONTRIBUTING.md Outdated
Comment on lines 31 to 33
### Governance Model

Our Governance model helps outline our project's decision making and roles-based expectations. Read more in our [GOVERNANCE.md](GOVERNANCE.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

There is no GOVERNANCE.md file included in the PR? I also don't think I've ever seen one in a project.

Copy link

Choose a reason for hiding this comment

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

@gkjohnson - see rationale in articles like this one. We do have recommendations for a GOVERNANCE.md though we're not yet pushing out PRs with this just yet.

CONTRIBUTING.md Outdated
Comment on lines 9 to 21
### Developer Certificate of Origin (DCO)

To accept your contributions for any kind of patch, you'll want to:
1. Understand what a Developer Certificate of Origin is. See [this](https://wiki.linuxfoundation.org/dco) guide.
2. Read and agree to the [terms](https://developercertificate.org) of the Developer Certificate of Origin.
3. Remember to add your Sign-Off for each patch contribution you submit to our project via either:
1. By using the `-s` flag if using Git. See [these](https://git-scm.com/docs/git-commit#Documentation/git-commit.txt--s) instructions.
2. By including the following text in your patch contributions (i.e. pull requests)
```
Signed-off-by: Full Name <email>
```

Reviewers reviewing your patch will look for the sign-off before deciding to accept your contribution.
Copy link
Contributor

Choose a reason for hiding this comment

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

This not something I've ever had to do for a project or seen recommended. At worst I've seen automated requests to sign a CLA (ie for Google's Github repos) but git automatically appends the users name and email that's set up through git to all commits.

Copy link

Choose a reason for hiding this comment

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

I've seen this used less too - in fact SLIM had a recommendation guide for setting up a GH Action to scan for signed commits that we later retracted. I'll note your feedback but feel free to remove this!

@riverma
Copy link

riverma commented Jan 11, 2024

Great - thanks for info. I can look through the remaining files and take what's needed but generally the state of the repo is based on what I've found works for my active open source projects after working on them for 7 or 8 years.

I appreciate and am glad to see an effort to provide consistency across projects but If you're open to feedback I think a lot of what's included here is overly verbose and extremely bureaucratic looking. For the types of projects NASA AMMOS open sources I think it's best to make it as easy as possible to contribute and not turn potential users and contributors away.

The "CONTRIBUTING.md" document in particular is extremely long and not something I would ever read before contributing to a project. It should be short and answer necessary questions people have before contributing to a project but not a lot more. Currently it includes a lot of basic Git explanations and complex rules that I've never seen properly followed in any project I've worked on (commit message requirements, sign offs, etc). They're nice in theory but it don't work in practice. In the worst case these kinds of documents and wording can scare people away from contributing to a project - which I've seen happen with NASA AMMOS projects in the past.

I'm happy to talk about these things more if you'd like.

@gkjohnson - appreciate your comments! Especially about making certain files crisper and more to the point. The SLIM project is open source and community run - we always welcome iterations and improvements. Would you be interested in helping improve the CONTRIBUTING.md guide based on your suggestions? It's as simple as proposing a PR here. Myself and the community can help iterate.

@gkjohnson
Copy link
Contributor

@riverma

Would you be interested in helping improve the CONTRIBUTING.md guide based on your suggestions? It's as simple as proposing a PR here.

Right now I'm inclined to not add a CONTRIBUTING.md file at all. The proposed one is too long and verbose to be useful, in my opinion. My feeling is that a file like this be added based on what the community seems to be struggling with or thinks is unclear. Preempting that with documentation of what should be considered common sense in a write up I think just makes the file less useful and a maintenance burden - which I'd generally like to keep to a minimum unless a team of people is going to consider themselves responsible for portions of the project long term.

I'm happy to add an adjusted pull request template and the CODE_OF_CONDUCT.md file from this PR, though.

@riverma
Copy link

riverma commented Jan 17, 2024

@gkjohnson - please feel free to adjust this PR based on your needs. We don't currently require, we simply suggest. If you have further thoughts or recommendations for adjusted files you think will benefit other projects, please again feel free to contribute to SLIM (NASA-AMMOS.github.io/slim)

@jpl-jengelke jpl-jengelke force-pushed the slim_infusion branch 2 times, most recently from b9b5843 to dd9163e Compare January 19, 2024 00:02
@gkjohnson
Copy link
Contributor

gkjohnson commented Jan 20, 2024

I'll add the manually instead of adjusting this PR.

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.

3 participants