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

First pass adding basic prerequisites #296

Merged
merged 12 commits into from
Jun 13, 2023
Merged

Conversation

samcunliffe
Copy link
Member

@samcunliffe samcunliffe commented May 15, 2023

Very basic preliminaries. CMake, Windows SDK, Xcode etc.

Opening early in case anyone has time to scrutinise this. Still waiting for some feedback from @prmunro 's postdoc and should also sanity-test these on a Windows machine.

Now tested on Windows, and I claim: ready for review.

I'll wait for Ed to email back some comments before merging (if he has a GH account he could even be invited to the review). This is probably good enough to go in around the same time as #320. Then we can patch.

@samcunliffe
Copy link
Member Author

samcunliffe commented May 16, 2023

TODO

  • Is MinGW better/easier?
    • is MinGW better? [ No, and our users use PowerShell + MSBuild.exe ]

@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Patch and project coverage have no change.

Comparison is base (b2396aa) 26% compared to head (a5ef852) 26%.

Additional details and impacted files
@@         Coverage Diff         @@
##           main   #296   +/-   ##
===================================
  Coverage    26%    26%           
===================================
  Files        83     83           
  Lines      3687   3687           
===================================
  Hits        949    949           
  Misses     2738   2738           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Very basic preliminaries. CMake, Windows SDK, Xcode etc.
* V. minor tweak to language.
* Add OS logos from EgoistDeveloper/operating-system-logos
* Split MacOS into its own thing.
@samcunliffe samcunliffe force-pushed the 292-pedagogical-prerequisites branch from e55f9b6 to f34f406 Compare May 17, 2023 14:18
@samcunliffe samcunliffe linked an issue May 18, 2023 that may be closed by this pull request
2 tasks
@samcunliffe samcunliffe marked this pull request as ready for review May 26, 2023 09:12
@samcunliffe samcunliffe added documentation Improvements or additions to documentation technical Technical and meta issues, not related to physics but infrastructure. review required A review approval is required before merging labels May 26, 2023
@samcunliffe samcunliffe added this to the End of ARC project. milestone May 26, 2023
Copy link
Collaborator

@willGraham01 willGraham01 left a comment

Choose a reason for hiding this comment

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

Approving, but obviously still pending feedback from Peter's postdoc/PhDs

README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
Co-authored-by: Will Graham <[email protected]>
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
PS> conda list fftw # assuming you installed via conda
PS> which.exe MATLAB
```
Which should return something like `C:\Program Files (x86)\MATLAB\R20XXx\bin\matlab` and maybe `C:\ProgramData\envs\base\bin`.
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
Which should return something like `C:\Program Files (x86)\MATLAB\R20XXx\bin\matlab` and maybe `C:\ProgramData\envs\base\bin`.
Which should return something like `C:\Program Files (x86)\MATLAB\R20XXx\bin\` for MATLAB, and `C:\ProgramData\anaconda\envs\base\Library\lib` or `C:\Users\<your username>\anaconda\Library\lib\cmake\fftw3`.

README.md Outdated Show resolved Hide resolved
</details>

<details>
<summary><img src="https://github.com/EgoistDeveloper/operating-system-logos/blob/master/src/24x24/WIN.png"/> Windows prerequisite setup</summary>
Copy link
Member Author

Choose a reason for hiding this comment

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

Tagging @edggjames FYI... 👇

Add comments to the README for when #181 is finished. Add instructions
to the dev docs (since devs might be compiling the HEAD of main.
@samcunliffe
Copy link
Member Author

@willGraham01 sorry for the noise. I've just made a (final??!) pass. Would you mind re-checking everything for me please?

@samcunliffe samcunliffe added this pull request to the merge queue Jun 13, 2023
Copy link
Collaborator

@willGraham01 willGraham01 left a comment

Choose a reason for hiding this comment

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

Won't pass explicit approval to prevent the merge queue drinking this up.

But looks fine, weird that the markdown section tags are rendering though. Ping me when things are addressed & I'll provide approval and get this into the queue.

<details>
<summary><img src="https://github.com/EgoistDeveloper/operating-system-logos/blob/master/src/24x24/WIN.png"/> Windows prerequisite setup</summary>

TDMS was developed on, and has been extensively tested on linux.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
TDMS was developed on, and has been extensively tested on linux.
TDMS was developed - and extensively tested - on linux.


Assuming you don't already have them, you'll need to download and install:

<!-- * HDF5 -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
<!-- * HDF5 -->
<!-- * TODO: When HDF5 comes in -->

Copy link
Member Author

Choose a reason for hiding this comment

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

Huh. I had an even better comment with a link to the page... lost.

doc/developers.md Show resolved Hide resolved
Merged via the queue into main with commit 47807b6 Jun 13, 2023
@samcunliffe samcunliffe deleted the 292-pedagogical-prerequisites branch June 13, 2023 12:12
@samcunliffe
Copy link
Member Author

I'll provide approval and get this into the queue.

Too much #DevelopmentVelocity. I'll send a followup PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation review required A review approval is required before merging technical Technical and meta issues, not related to physics but infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pedagogical instructions (+ specifically help for Windows users)
2 participants