-
Notifications
You must be signed in to change notification settings - Fork 16
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
26 - Add frontend guides #353
Conversation
…idea of information needed readme badge updates readme updating misc Readme updates
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ran out of steam once I hit the bit README but here's some quick feedback.
.github/SUPPORT.md
Outdated
|
||
# Looking for support? We want to help | ||
|
||
Please post your question to X (#) using the Timber tag. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this X/Timber stuff?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the SUPPORT.md file because the file seemed like a template with no real information, and we already cover the various channels through which people can contact us in the README, such as Google Groups, Zulip, etc.
.github/SUPPORT.md
Outdated
|
||
## Other places | ||
|
||
You can use [GitHub Discussions](#) to open up questions and engage in discussions with the contributors and other users. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we plan to use GitHub Discussion? That's fine. I'm just curious. 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I asked the frontend team, and for the moment, we'll stick with Zulip.
.github/SUPPORT.md
Outdated
|
||
You can use [GitHub Discussions](#) to open up questions and engage in discussions with the contributors and other users. | ||
|
||
Please don't post to the [EXAMPLE forum](#). You might see a response, but it will just redirect you to either GitHub (for bugs/issues) or StackOverflow (for usage/support questions). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need this line? Don't post somewhere? Stackoverflow? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the SUPPORT.md file because the file seemed like a template with no real information, and we already cover the various channels through which people can contact us in the README, such as Google Groups, Zulip, etc.
AUTHORS.md
Outdated
# Thanks to everyone for your contributions! | ||
|
||
Harvard University IQSS Team | ||
FirstName LastName <[email protected]> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm just curious where this AUTHORS file idea came from. I do see Sinatra uses it according to https://opensource.guide/building-community/#share-ownership-of-your-project via https://stackoverflow.com/questions/18459504/what-is-the-origin-and-purpose-of-authors-md
I'm not opposed to it. Again, just curious.
There's also CODEOWNERS: https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners
And there's All Contributors, which @poikilotherm is playing around with here:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I removed the file for now since it was just a template that needs to be filled with real information.
There's an issue to work on it:
However, at the moment, it doesn't seem like a critical file in the Frontend Guides, and we have some other issues related to the frontend guides in this Sprint with higher priority than the AUTHORS file.
CHANGELOG.md
Outdated
Resources for generating a changelog: | ||
|
||
[skywinder/Github-Changelog-Generator](https://github.com/skywinder/Github-Changelog-Generator) - generates a full changelog that overwrites the existing CHANGELOG.md. | ||
|
||
[hzalaz/wt-oss-milestone-changelog](https://github.com/hzalaz/wt-oss-milestone-changelog) - generates a snippet of Markdown that can be added to a CHANGELOG.md. | ||
|
||
[conventional-changelog/conventional-changelog](https://github.com/conventional-changelog/conventional-changelog/tree/master/packages/conventional-changelog-cli) - generates a full changelog based on commit history with the option to append to an existing changelog. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest moving these elsewhere for the final changelog.
There's also https://keepachangelog.com (not for generating, but just a high level of why a changelog is a good idea).
CODE_OF_CONDUCT.md
Outdated
@@ -0,0 +1,142 @@ | |||
# Dataverse® Code of Conduct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the Code of Conduct be the same as https://github.com/IQSS/dataverse/blob/develop/CODE_OF_CONDUCT.md ? (It's what we link to from https://dataverse.org as well.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done! I replaced it with the CODE_OF_CONDUCT from the main Dataverse repo
CONTRIBUTING.md
Outdated
|
||
## Documenting | ||
|
||
This is probably one of the most important things you can do as a contributor and probably one of the easiest things you can do. There are two big pieces of documentation you can edit right here on github! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably one of the most important things you can do as a contributor and probably one of the easiest things you can do. There are two big pieces of documentation you can edit right here on github! | |
This is probably one of the most important things you can do as a contributor and probably one of the easiest things you can do. There are two big pieces of documentation you can edit right here on GitHub! |
CONTRIBUTING.md
Outdated
|
||
Hey there! We’re really happy you’ve found your way here. Dataverse® is a community project that is developed by people from all over the world. We appreciate any help. | ||
|
||
Before you start working on something we would suggest talking to us in the [TODO: discussion forum](#) or asking a question on [TODO: GoogleGroups/Zulip](#). You can also contact us via the chat widget on the [TODO: main Dataverse website](#). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pointing out that there are a number of TODO's in the doc, including this one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I addressed all the TODOs in the CONTRIBUTING.md file. Well, I mostly rewrote it, taking the CONTRIBUTING.md file from the main Dataverse repository as a reference. I also added the DEVELOPER_GUIDE.md file for more information about code contributions
CONTRIBUTING.md
Outdated
|
||
## Ownership | ||
|
||
By contributing code you are granting _(`Dataverse`)_ shared ownership of your work. You still own it but _(`Dataverse`)_ will have the right to relicense your work based on our needs & treat this work as if it was developed by a _(`Dataverse`)_ engineer. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this legalese correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No idea, I just removed that part and used the Dataverse main repository's CONTRIBUTING file as a reference.
Moving this back to in progress because there are a number of TODOs |
…st-cases 326 - Developer guide - Writing test cases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I just have a few comments
…into docs/26-add-frontend-guides
@ekraffmiller thanks for your comments. I addressed the changes! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @MellyGray for your changes, I just found one small thing from the merge to look at
README.md
Outdated
<<<<<<< HEAD | ||
|
||
<p align="right">(<a href="#readme-top">back to top</a>)</p> | ||
<br> | ||
======= | ||
We have introduced an update to the breadcrumb navigation UI. Unlike in the original JSF application, where breadcrumbs | ||
did not reflect the user's current location within the site, our new SPA design now includes this feature in the breadcrumbs. | ||
Additionally, we have aligned with best practices by positioning all breadcrumbs at the top, before anything else in the UI. | ||
>>>>>>> 3aa4685da974fdc9328c44866237bd8173e31cda |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like this is from a merge conflict, can it be deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good thing you noticed that! Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
26 - Add frontend guides
What this PR does / why we need it:
This issue adds a basic Frontend Guide. Which will be extended in future issues to add different sections in more detail.
Which issue(s) this PR closes:
Special notes for your reviewer:
This issue was started by Mathew, but he left the project midway, so I finished some of the TODOs.
Suggestions on how to test this:
Visual inspection of the changed files from the README
Does this PR introduce a user interface change? If mockups are available, please link/include them here:
No
Is there a release notes update needed for this change?:
New Frontend Guide
Additional documentation: