Refactor Decision Record directory structure and file names #8
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
@vraer In order to review your changes, I need to update my forked main branch locally. However, I am running into these errors I think are caused by having a colon in the file name: Click here to see screenshot of the error
I'm using Windows 11. Did you encounter this error when you tried updating your forked website-wiki's main branch locally? If yes, how did you fix the error? |
|
@JessicaLucindaCheng I've removed all the colons from the original DR filenames in the upstream repo's main branch. Syncing your forked main branch should resolve the error. Let me know if you still have trouble! |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Notes for myselfI finally figured out how to view the changes but it required a workaround and may not be how we want junior developers to review changes in prs. In ERs or maybe a discussion or both, which I will write up, I think the following needs to happen:
Also,
I think I will finally be able to review the changes tomorrow. Thank you for your patience while I worked through my problems.
|
There was a problem hiding this comment.
@vraer Thanks for all your hard work on the wiki! I liked the structure and clear instructions for the Decision Records Management page.
- Since I'm not sure what discussions you have had with Bonnie, if anything I ask you to change contradicts what Bonnie has said, just follow what Bonnie has said. Also, let me know so that I don't ask you to make those changes in the future.
- Also, I wasn't sure if you were supposed to fix problems with links, grammar, spelling, etc, or if that is out of the scope of what you are doing. If any changes I suggested are out of the scope of what you are supposed to be doing right now, let me know and I'll write an ER so that issues can be made to fix them.
- Edit: Just spoke with Bonnie and got a better understanding of the scope of what you are doing, which is just to get the website-wiki up and running so it is useable and we can start adding stuff to it again. Thus, any changes that I am requesting outside of that scope, you can just leave a comment, "Out of scope" and leave the requested change unresolved. I'll go through anything unresolved and make sure that eventually there are issue(s) made from it to fix the things.
In addition to my in-line comments, please do the following as well:
- In your original PR comment, please add
Fixes https://github.com/hackforla/website/issues/4363so it links back to the issue since you added the updates to that decision records as part of this pr. - For
docs/DR/adopted/use-merge-commit-for-gh-pages-updates.md, can the title be changed from "Use merge commit for gh pages updates" to "Update feature branches using merge commit"? The reason is the current title might be confused with updating the gh-pages branch when the decision record is actually about updating the feature branches. - In the docs/DR/adopted/update-gha-with-dependabot.md, under the Feasibility Determination section, in the paragraph that starts with
Notes:, changetoThe issue to create the [file](https://github.com/hackforla/website/issues/3843)Issue [#3843](https://github.com/hackforla/website/issues/3843) creates the file for the website repo. - In
docs/DR/index.md, I noticed the titles of the decision records are based on the filename with the first letter capitalized but for the Decision Records Management page you manually added each file and had different capitalization. You may want to consider using atitle:meta-key for each page. For more info on how MkDocs does titles, see Setting the page title and "title" in the "Meta-Data" section.- Pros of doing this are
- This will allow you to have more control over the capitalization and titles independent of the filename.
- This will also make the capitalization the same in the navigation on the left side of the website.
- Cons of doing this are
- If the wording of the title is different than the filename, the left navigation will list it alphanumerically by its filename and not its title.
- Pros of doing this are
- Can the directory
DRbe renamed toDecision Records? Or at least have the directory name be displayed asDecision Records? I'm not sure a new team member will know what DR stands for.
|
|
||
| #### Potential Solution | ||
|
|
||
| Use the img HTML tag without an ending slash meaning <img...> (Source 1). |
There was a problem hiding this comment.
Put <img...> in backticks otherwise it doesn't show up on the page since it is read as an actual tag. See line 13 in this file for an example of <img...> in backticks.
| #### Summary | ||
|
|
||
| - Potential linters for use on developers' local machines were researched as requested by issue #2651 and ESLint was suggested as a possible solution, however a decision cannot be made until a spell checker for the repo is decided on so that we can determine compatability. | ||
| - If the decision to implement ESLint as the standard local linter were made, we would need to add the files it creates to the main repo; a package.json and .eslintrc.yml config file and we would also have to implement ESLint onto the repo with Github actions so that the repo linter matches everyone's local linter. |
There was a problem hiding this comment.
Change the colon in main repo; a package.json and .eslintrc.yml to a comma so the result is main repo, a package.json and .eslintrc.yml. The punctuation didn't make sense to me with the colon, so I think it is supposed to be a comma.
|
|
||
| **Potential Solution** | ||
| #### Potential Solution | ||
|
|
||
| Using Dockers to check to see if the button has been hidden and the button is still within toolkit.html |
There was a problem hiding this comment.
Change Dockers to Docker
|
|
||
| #### Problem Statement | ||
| Per comment, it was proposed that a list of "good second issues" be made to add the hyperlinks to each of the projects in the .md files. Before this issue's linked pull request, the project names on the mini cards led to their hackforla.org/projects/.. page. After the pull request was merged, the project names led to their respective hackforla github pages. | ||
|
|
||
| Per comment, it was proposed that a list of "good second issues" be made to add the hyperlinks to each of the projects in the .md files. Before this issue's linked pull request, the project names on the mini cards led to their hackforla.org/projects/.. page. After the pull request was merged, the project names led to their respective hackforla github pages. |
There was a problem hiding this comment.
Change github pages to GitHub pages
| #### Problem Statement | ||
| Over time, feature branches become out of date with the code in the gh-pages branch. After updating the feature branch as outlined in steps [A. Updating a feature branch with changes from gh-pages](How-to-update-a-feature-branch-with-changes-from-gh-pages#a-updating-a-feature-branch-with-changes-from-gh-pages) and [B. Opening a pull request](How-to-update-a-feature-branch-with-changes-from-gh-pages#b-opening-a-pull-request) from [How to update a feature branch with changes from gh pages](How-to-update-a-feature-branch-with-changes-from-gh-pages) wiki page, we need to figure out the best way to merge an "update the feature branch" pull request into the feature branch. (An "update the feature branch" pull request means a pull request that is opened for updating a feature branch with changes from gh-pages branch.) | ||
|
|
||
| Over time, feature branches become out of date with the code in the gh-pages branch. After updating the feature branch as outlined in steps [A. Updating a feature branch with changes from gh-pages](https://github.com/hackforla/website/wiki/How-to-update-a-feature-branch-with-changes-from-gh-pages#a-updating-a-feature-branch-with-changes-from-gh-pages) and [B. Opening a pull request](https://github.com/hackforla/website/wiki/How-to-update-a-feature-branch-with-changes-from-gh-pages#b-opening-a-pull-request) from [How to update a feature branch with changes from gh pages](https://github.com/hackforla/website/wiki/How-to-update-a-feature-branch-with-changes-from-gh-pages) wiki page, we need to figure out the best way to merge an "update the feature branch" pull request into the feature branch. (An "update the feature branch" pull request means a pull request that is opened for updating a feature branch with changes from gh-pages branch.) |
There was a problem hiding this comment.
-
The links in this paragraph need to be updated to link to the MkDocs documentation instead of the old wiki. Though, I'm not sure if you are ready to do this yet or if you plan on doing it later because I don't know what your plan is for changing links pointing to the old wiki to the new wiki.
-
In the last sentence
(An "update the feature branch" pull request means a pull request that is opened for updating a feature branch with changes from gh-pages branch.)
add
thetofrom gh-pages branchso the result isfrom the gh-pages branch.
| Although the above could be a solution, I was not able to recreate the bug. In addition, the bug occurred within a 2 week period and has yet to occur again in 4 months. There's a possibility the bug has already been squashed. | ||
|
|
||
| Here's a summary of my findings. | ||
|
|
||
| - There are other reasons that labels aren't added by github-actions: | ||
| - merge conflict (any on: pull_request workflows will not be triggered) | ||
| - not merging into the specified branches (this can be found in the workflows .yml file) |
There was a problem hiding this comment.
Change workflows .yml file to workflows' .yml files
| @@ -39,5 +48,3 @@ If the bug comes up again, start to find the bug ASAP! Here are some tests to do | |||
|
|
|||
|  | |||
| </details> | |||
There was a problem hiding this comment.
The following requested changes are to 1) clarify what each of the figures show and 2) fix the images so they show correctly on the website.
- In lines 43-45, change the code to
<details><summary>Figure 1: Code snippet of the branches the Set PR Labels workflow will work on</summary> <img src="https://user-images.githubusercontent.com/31293603/199852407-56a82546-7a13-47ca-9253-b68fe618c487.png" alt=""> - In lines 47-49, change the code
<details><summary>Figure 2: Where to change your default branch in your repo</summary> <img src="https://user-images.githubusercontent.com/86996158/179142211-04764947-c20a-4187-898a-d911d6f196f4.png" alt="">
| [#2978](https://github.com/hackforla/website/issues/2978) | ||
| [Guides Issue #10 Comment](https://github.com/hackforla/guides/issues/10#issuecomment-1066190743) |
There was a problem hiding this comment.
Make these into bullet points to make it easier to read, so the result is
- [#2978](https://github.com/hackforla/website/issues/2978)
- [Guides Issue #10 Comment](https://github.com/hackforla/guides/issues/10#issuecomment-1066190743)
|  | ||
|
|
||
| Actual Output | ||
|  | ||
|
|
||
| </details> | ||
|
|
||
| <details> <summary> Backslashes appear randomly in the file </summary> | ||
| <details> <summary> Backslashes appear randomly in the file </summary> | ||
|
|
||
|  |
There was a problem hiding this comment.
For the image to display, change this to use an img HTML tag, so the result is
<img src="https://user-images.githubusercontent.com/38295612/158070915-a6937dde-5b84-46cc-b697-225b2b14021b.png" alt="">
| Expected Output | ||
|  | ||
|
|
||
| Actual Output | ||
|  |
There was a problem hiding this comment.
The images need to be put in img HTML tags so the images display properly. The "Expected Output" and "Actual Output" needs to be formatted so it isn't inline with the images. You could do the following but feel free to change the formatting if you want:
<b>Expected Output</b><br>
<img src="https://user-images.githubusercontent.com/38295612/158072463-0497a1f4-88e7-4fe5-a22f-80eeed20f60c.png" alt="">
<br><br>
<b>Actual Output</b><br>
<img src="https://user-images.githubusercontent.com/38295612/158072457-1fd97035-a67c-41a3-a221-780b91fa8edf.png" alt="">
There was a problem hiding this comment.
@vraer Thanks for all your hard work on the wiki! I liked the structure and clear instructions for the Decision Records Management page.
- Since I'm not sure what discussions you have had with Bonnie, if anything I ask you to change contradicts what Bonnie has said, just follow what Bonnie has said. Also, let me know so that I don't ask you to make those changes in the future.
- Also, I wasn't sure if you were supposed to fix problems with links, grammar, spelling, etc, or if that is out of the scope of what you are doing. If any changes I suggested are out of the scope of what you are supposed to be doing right now, let me know and I'll write an ER so that issues can be made to fix them.
In addition to my in-line comments, please do the following as well:
- In your original PR comment, please add
Fixes https://github.com/hackforla/website/issues/4363so it links back to the issue since you added the updates to that decision records as part of this pr. - For
docs/DR/adopted/use-merge-commit-for-gh-pages-updates.md, can the title be changed from "Use merge commit for gh pages updates" to "Update feature branches using merge commit"? The reason is the current title might be confused with updating the gh-pages branch when the decision record is actually about updating the feature branches. - In the docs/DR/adopted/update-gha-with-dependabot.md, under the Feasibility Determination section, in the paragraph that starts with
Notes:, changetoThe issue to create the [file](https://github.com/hackforla/website/issues/3843)Issue [#3843](https://github.com/hackforla/website/issues/3843) creates the file for the website repo. - In
docs/DR/index.md, I noticed the titles of the decision records are based on the filename with the first letter capitalized but for the Decision Records Management page you manually added each file and had different capitalization. You may want to consider using atitle:meta-key for each page. For more info on how MkDocs does titles, see Setting the page title and "title" in the "Meta-Data" section.- Pros of doing this are
- This will allow you to have more control over the capitalization and titles independent of the filename.
- This will also make the capitalization the same in the navigation on the left side of the website.
- Cons of doing this are
- If the wording of the title is different than the filename, the left navigation will list it alphanumerically by its filename and not its title.
- Pros of doing this are
- Can the directory
DRbe renamed toDecision Records? Or at least have the directory name be displayed asDecision Records? I'm not sure a new team member will know what DR stands for.
There was a problem hiding this comment.
@vraer I'm going to go ahead and approve this pr because
- the changes requested can be made later and
- I don't want to hold up the progress on the website-wiki since Bonnie wants the website-wiki ready as soon as possible for new submissions or edits by other team members.
Thanks for all your hard work on this wiki migration!
|
@JessicaLucindaCheng FYI, I implemented all of your suggestions in my latest updates to the decision records. Could use some guidance on how to best group all of these commits/PR(s) Let me know if you're up for that discussion :) |
Part of this pr fixes hackforla/website#4363
This pull request introduces significant updates to the Decision Record (DR) directory in the
website-wikirepository. The changes aim to improve the organization and readability of decision records.Key Changes:
New Folder Structure for Decision Records: A new folder structure has been implemented for the decision records. Two new folders,
adoptedandnot_implemented, have been created to categorize the decision records based on their implementation status.Renaming of Files in DR Directory: The files in the DR directory have been renamed for clarity and consistency. The new naming convention removes the
DR:prefix, replaces spaces with hyphens, converts the title to lowercase, and makes minor changes to the file name to avoid special characters.Content Updates: New content has been added to the
decision-records-management.mdandindex.mdfiles. These updates provide more context and guidance for managing and navigating the decision records. Specifically,decision-records-management.mdnow houses the necessary templates and instructions for creating new decision records, whileindex.mdcontains a list of all the decision records grouped byadoptedandnot_implemented..pages File Addition: A
.pagesfile has been added, utilizing the recently added AwesomePages plugin to organize how the DR directory appears in the wiki's overall navigation.