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

Clarify FLINT prerequisites #58

Merged

Conversation

sarahhaggarty
Copy link
Contributor

@sarahhaggarty sarahhaggarty commented Jul 15, 2021

Description

This PR is a proposal to make changes to the existing documentation to make it more explicit to the reader when to fork and clone FLINT core repository as part of the installation process. For further details and motivation please read: #57

Fixes #57

Type of change

Please delete options that are not relevant.

  • This change is a documentation update

How Has This Been Tested?

N/A

Additional Context (Please include any Screenshots/gifs if relevant)

...

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • I have checked my code and corrected any misspellings
  • I have signed the commit message to agree to Developer Certificate of Origin (DCO) (to certify that you wrote or otherwise have the right to submit your contribution to the project.) by adding "--signoff" to my git commit command.

@sarahhaggarty
Copy link
Contributor Author

Hi @HarshCasper @shubhamkarande13 @aornugent,

Would you be able to review this PR?

@aornugent aornugent requested review from shubhamkarande13 and removed request for aornugent July 19, 2021 10:38
@aornugent
Copy link
Contributor

Could you please take a look at this @shubhamkarande13 - it should be very familiar after creating your videos!

Amended docs to be more explicit that FLINT core repository is needed before proceeding to FLINT.example

Signed-off-by: sarahhaggarty <81160244+sarahhaggarty@users.noreply.github.com>
Remove instruction to build visual studio solution from vcpkg prerequisites page as it is too early
Signed-off-by: sarahhaggarty <81160244+sarahhaggarty@users.noreply.github.com>
@sarahhaggarty
Copy link
Contributor Author

Hi @shubhamkarande13 @chicken-biryani, thank you so much for your time earlier this week to discuss this PR. It was really helpful! As we discussed, I created two new issues: #59 and #60.

I reverted my suggested change about moving "Data Sets" to FLINT.example from this PR so that issue can be followed instead on #60.

This leaves the following changes that we spoke about:

  • Removed the statement about FLINT.example completely in DevelopmentSetup/index.rst so that readers will continue to the next page which is the "Git and Github Guide" where they are instructed to fork and clone FLINT core repository. This will prevent them from making the same mistake that I made.
  • Amended the statement in DevelopmentSetup/FLINT.example_installation.rst to: "It is recommended to practice running simulations using FLINT.example repository after the FLINT core installation is completed. This will help you to familiarise yourself with FLINT and help you to understand how FLINT works."
  • Added in the prerequisites sections of the below pages that relevant FLINT core and FLINT.example repositories need to be forked and cloned before proceeding:
    DevelopmentSetup/windows_installation.rst
    DevelopmentSetup/visual_studio_win_example.rst
  • Removed the instruction in Prerequisites/vcpkg.rst to build the visual studio solution because this is too early.

I double-checked the videos and the changes I've proposed have no impact as the sections which are changed are not consulted or shown in the videos.

@shubhamkarande13 do you think you're OK to review this PR now? Let me know if there is anything I missed!

@shubhamkarande13
Copy link
Member

Great PR @sarahhaggarty!
Thank you so much for your hard work!

@shloka-gupta shloka-gupta merged commit c92d3a1 into moja-global:master Aug 1, 2021
@sarahhaggarty sarahhaggarty deleted the clarify-flint-prerequisite branch August 3, 2021 12:25
@HarshCasper
Copy link
Member

@all-contributors bot please add @sarahhaggarty for docs bug mentoring review

@allcontributors
Copy link
Contributor

@HarshCasper

I've put up a pull request to add @sarahhaggarty! 🎉

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.

Clarify statement about "setting up" FLINT.example before FLINT core
5 participants