Loris
Loris copied to clipboard
aces
[Documentation] Adding Developers Guide to docs
This document offers general guidance to developers (both inside and outside the LORIS team) on how to contribute to LORIS and the general workflow of LORIS code development. It is still in progress.
As of the latest commit, this doc was added to ReadTheDocs. To see a preview of how it renders, go to this link: https://loris-fork.readthedocs.io/en/2020-08-13-developersguidemarkdown/docs/wiki/99_Developers/Developers_Guide.html
TODO:
-
[x] Complete the Code Review section
-
[x] Add more detail to the Accessing a Database section
-
[x] Fix formatting and wording
@christinerogers Concerning your comment about re-ordering the sections, I have changed their order to what I think is most valuable for outside developers:
- Working off your fork, 2. Issues, 3. PRs, 4. Code Review, 5. Rebasing, 6. DB
@christinerogers I implemented the changes you requested above and also read through the document to check for tone issues. I removed a majority of the "Code Review Etiquette" section and moved it to the internal doc. I kept the "When to resolve a review or comment thread" content because I think that information is specific to how Loris handles code review. Let me know what you think!
@laemtl @h-karim @spell00 Could you read this over on Monday afternoon, and review it?
Keep in mind:
-
this doc contains a subset of points taken from the original CheatSheet we started in May -- specifically what's relevant/appropriate for developers who are not part of our MNI group but wish to either customize or contribute to LORIS. New devs joining the Loris team will also read this as a key doc during their onboarding. In parallel, new Loris team devs will also consult the remainder of the CheatSheet points (@AlexandraLivadas , can you share again in slack the updated link to this remaining internal CheatSheet?)
-
Assume this audience has 3 years experience knows already how to make informative pull requests etc. In most cases, their main priority is probably to customize their own instance Loris, not always to contribute to our project.
After a few of you have read it over and added your suggestions -- later next week @haoweiqiu will be reviewing it to verify that it makes sense to someone new to the project.
thanks @h-karim for reviewing
@laemtl @spell00 -- reminder could you please spend a few minutes reading/reviewing this? thanks !
@christinerogers What do you think about @laemtl's comment about adding this to ReadTheDocs?
@christinerogers What do you think about @laemtl's comment about adding this to ReadTheDocs?
Laetitia is 200% right - it should be in rendered in RTD.
@laemtl and @AlexandraLivadas -- Please loop in @haoweiqiu so he sees and can document for us internally what are the steps to create/update docs and render them in RTD.
The numbered list issue is still not fixed. One of this solutions should work though: https://stackoverflow.com/questions/18088955/markdown-continue-numbered-list/22138846#22138846
@christinerogers @driusan Do you mind re-reviewing with the updated RTD formatting? Here is the link to see the RTD view.
I tried to clarify the section. To remove the wiki link to https://github.com/aces/Loris/wiki/Code-Review-Checklist, this page needs to be migrated first.
@driusan This would be useful to merge for our new group of developers. Would you have a few minutes to take another look? Thanks --
currently blocking a task for our 2021 GSOC student, aces/Loris-MRI#562 this PR should be merged before he can begin.
@AlexandraLivadas could you please rebase so that Travis can run on it? Thank you!
@driusan @christinerogers This is ready for review. All links are working and I've read over the content carefully :) Here is a link to the ReadTheDocs
Other than the requested changes.... I'm not a big fan of the rebasing explanations partly because it's not our duty, these are not LORIS specific rebasing instructions, and partly because these instructions vary greatly from one case to another and from one developer to another... but I wont block over this
Hi @ridz1208! The rebasing instructions were added since this document is meant to cover all topics related to a normal Loris workflow for contributing developers. I do see your point though that the rebasing tips provided can be found elsewhere and aren't Loris specific, so I see no issue in removing this section. @christinerogers Is this okay with you?
@cmadjar @driusan I just realized this is waiting for confirmation from @christinerogers
Hi @ridz1208! The rebasing instructions were added since this document is meant to cover all topics related to a normal Loris workflow for contributing developers. I do see your point though that the rebasing tips provided can be found elsewhere and aren't Loris specific, so I see no issue in removing this section. @christinerogers Is this okay with you?
Hi Alex -- Can you move these please to the Internal LORIS Cheat sheet? Thanks -- likewise for anything the senior team wants to cut (or not publish), just move it to the Loris cheat sheet. thanks!
@AlexandraLivadas take a look at dave's change requests and i'll approve when addressed
@cmadjar is it time to bump this for review? This should be merged for the release. (i can add it to the team agenda too)
@christinerogers I think this is more a question for @driusan or @ridz1208. I am mainly the note taker when I assign on GitHub, lol
oh whoa this PR is still open. Dave's change request is still outstanding :
Still referencing Travis.
- @AlexandraLivadas can you resolve this quickly or would it take time?
@driusan @ridz1208 could this make it into the next minor release ? I'll tag it 24 in case that helps.
@driusan I just fixed the conflict! The mkdocs version in readthedocs/requirements.txt has been updated since this PR was made so the conflict was in that file.