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

Provide diff with all changes in step-by-step example. #262

Merged
merged 26 commits into from
Nov 14, 2023

Conversation

BenjaminRodenberg
Copy link
Member

@BenjaminRodenberg BenjaminRodenberg commented Apr 22, 2023

I just prepared this for my thesis and thought it might also be useful for the documentation. It's unfortunately quite lenghty, but there are also many changes. I'm open for suggestions!

Todo

  • Split code block. See 74e377c. @uekerman , @fsimonis any guidelines? Does not look critical to me. We can also fix this later, if necessary.

Copy link
Member

@uekerman uekerman left a comment

Choose a reason for hiding this comment

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

I like this idea. My suggestion would be to move this code part at the bottom and explain all individual steps one-by-one above (not in this PR).

No idea about the code split. Seems like we should do this.

@MakisH
Copy link
Member

MakisH commented Apr 28, 2023

Related to #260

Copy link
Member

@fsimonis fsimonis left a comment

Choose a reason for hiding this comment

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

Can't we changes these to std::vector now?

pages/docs/configuration/configuration-action.md Outdated Show resolved Hide resolved
@BenjaminRodenberg BenjaminRodenberg added this to the Version 3.0.0 milestone Nov 11, 2023
Copy link
Member

@MakisH MakisH left a comment

Choose a reason for hiding this comment

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

Looks really nice, and very helpful, thank you!
I have not cross-checked with the latest state of the API, but the changes make sense compared to what I remember from the last time I checked.

@@ -16,6 +16,91 @@ Please add breaking changes here when merged to the `develop` branch.

## preCICE API

<!-- Split code block. See https://github.com/precice/precice.github.io/commit/74e377cece4a221e00b5c56b1db3942ec70a6272. -->
Copy link
Member Author

Choose a reason for hiding this comment

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

@MakisH Do you have more details on this point? If it is simple: Let's do it now. If it is more complicated: I guess it's not critical and we can just merge.

Copy link
Member

Choose a reason for hiding this comment

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

I do not understand what you mean. Could you please elaborate what the problem is? I vaguely remember some issue with code highlighting in diff. Normal diff should work.

Copy link
Member Author

Choose a reason for hiding this comment

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

According to 74e377c there seems to be some issue with longer code blocks. But let's not worry about it here. I'll open an issue, because we should probably discuss this with @chlorenz and add some general information and guidelines somewhere in the documentation for the documentation.

@BenjaminRodenberg BenjaminRodenberg merged commit cb25b14 into master Nov 14, 2023
2 checks passed
@BenjaminRodenberg BenjaminRodenberg deleted the porting-summarize-diff branch November 14, 2023 11:16
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.

4 participants