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

Add CLion IDE developer documentation #3045

Merged
merged 13 commits into from
Jan 10, 2024

Conversation

JanVogelsang
Copy link
Contributor

@JanVogelsang JanVogelsang commented Dec 11, 2023

This PR adds developer documentation on how to setup the CLion IDE for NEST, which is a popular commercial IDE for C++ projects.

[edit] review docs here

@JanVogelsang JanVogelsang added T: Enhancement New functionality, model or documentation S: Normal Handle this with default priority labels Dec 11, 2023
@JanVogelsang JanVogelsang self-assigned this Dec 11, 2023
Copy link
Contributor

@jessica-mitchell jessica-mitchell left a comment

Choose a reason for hiding this comment

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

thanks for adding this. However, I find the instructions not clear enough and I don't think all these screen shots are necessary. i would suggest being more explicit in the text and only keeping the run configuration screen shot and maybe the cmake one.
For example
" In the left panel select Build,Execution,Deployment -->CMake. Then add CMake options . . ."
" Navigate to the ??? to edit the run configuration. "
Then explain which items need to be edited (e.g., "Change the working directory to the path to the source directory for NEST" ).

@JanVogelsang
Copy link
Contributor Author

Alright, I addressed your comments and made the instructions clearer. @otcathatsya Could you have a look at this as well and tell me if these instructions are clear enough?

@JanVogelsang
Copy link
Contributor Author

It doesn't look very pretty yet, for some reason all lines that are followed by an unordered list are bold. I couldn't find out how to fix that, could you do that Jessica?

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

Some of the instructions are horribly outdated, e.g., mention OpenMPI 1.6 and MacPorts, which hardly anyone uses any more. I will try to take a look at the Mac part.

@otcathatsya
Copy link
Contributor

Alright, I addressed your comments and made the instructions clearer. @otcathatsya Could you have a look at this as well and tell me if these instructions are clear enough?

I think to a dev using CLion it's pretty clear, though maybe important to state that most of the steps described are to be able to properly debug NEST with the built-in IDE tools, since the "regular" project will work pretty much out of the box from my experience.

@JanVogelsang
Copy link
Contributor Author

Some of the instructions are horribly outdated, e.g., mention OpenMPI 1.6 and MacPorts, which hardly anyone uses any more. I will try to take a look at the Mac part.

Alright, please open a separate issue (or ideally also a PR) for that, as this is unrelated to the documentation for using CLion.

@JanVogelsang
Copy link
Contributor Author

Alright, I addressed your comments and made the instructions clearer. @otcathatsya Could you have a look at this as well and tell me if these instructions are clear enough?

I think to a dev using CLion it's pretty clear, though maybe important to state that most of the steps described are to be able to properly debug NEST with the built-in IDE tools, since the "regular" project will work pretty much out of the box from my experience.

Right, that's true. The run configuration is required to debug C++ with CLion and the CMake configuration is fully optional, but helps with two things: 1. If not properly setup (e.g. just leaving the defaults), CLion reruns CMake from time to time (I think when checking out a new branch for example) and thus overwrites the files one generated when manually running CMake from the command line. 2. Even though I personally don't do that, one does not need the terminal at all anymore, as cmake, make, make install, and python .. are all run from CLion itself now.
I'll add that to the documentation.

@terhorstd
Copy link
Contributor

Note that I added a link to review the rendered documentation into the PR description.

@JanVogelsang
Copy link
Contributor Author

Note that I added a link to review the rendered documentation into the PR description.

Will this be automatically rebuild now that I made some more changes?

@terhorstd terhorstd added the I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) label Dec 15, 2023
@jessica-mitchell
Copy link
Contributor

@jessica-mitchell
Copy link
Contributor

It doesn't look very pretty yet, for some reason all lines that are followed by an unordered list are bold. I couldn't find out how to fix that, could you do that Jessica?

This is because you need new lines around your second order bullet lists. Also in the first paragraph, you are missing a new line before the bullet list so the bullets are ignored.

Good rule of thumb in reStructured text is always include new lines around markup for headings, lists, directives etc.

@JanVogelsang
Copy link
Contributor Author

Alright, thanks, it looks good now.

Copy link
Contributor

@heplesser heplesser left a comment

Choose a reason for hiding this comment

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

@JanVogelsang I see your point about not fixing outdated, unrelated material. I will look at that in a separate PR. Please see the suggestion I added on CLion.

Co-authored-by: Hans Ekkehard Plesser <hans.ekkehard.plesser@nmbu.no>
@jessica-mitchell
Copy link
Contributor

@JanVogelsang I wanted to create a PR against your branch because I had a couple of changes I was trying out myself. But your repo isn't listed in the base repositories. This happens quite frequently, and I thought it might be because of private repos but yours is public. Do you happen to know why this is?
These are the changes I wanted to make. Could you apply the commit to your branch?

@JanVogelsang
Copy link
Contributor Author

@jessica-mitchell No idea why that is the case, but sure, I can apply the commit.

@jessica-mitchell jessica-mitchell merged commit c2a3188 into nest:master Jan 10, 2024
24 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I: No breaking change Previously written code will work as before, no one should note anything changing (aside the fix) S: Normal Handle this with default priority T: Enhancement New functionality, model or documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants