Expanding guidelines for contributions - [merged] #1540

Closed SGeeversAtVortech closed 3 weeks ago

In GitLab by @jarsarasty on Oct 24, 2023, 12:01

_Merges guidelinescontributions -> master

This commit extends the guidelines for community contributions to RTC tools. It defines guidelines for creating issues and merge requests, and explains the procedure for submitting changes. This commit also includes an additional description of RTC-Tools in the Readme.md file.

In GitLab by @jarsarasty on Oct 24, 2023, 12:01

requested review from @Ailbhemit and @SGeeversAtVortech

In GitLab by @Ailbhemit on Oct 24, 2023, 12:07

Commented on CONTRIBUTING.md line 12

Should details be added for this section?

In GitLab by @jarsarasty on Oct 24, 2023, 13:41

Commented on CONTRIBUTING.md line 12

Good point! I will include some details here.

In GitLab by @jarsarasty on Oct 24, 2023, 13:59

Commented on CONTRIBUTING.md line 12

changed this line in version 2 of the diff

In GitLab by @jarsarasty on Oct 24, 2023, 13:59

added 1 commit

fa1d86c1 - Adding details to Improving documentation

Compare with previous version

In GitLab by @jarsarasty on Oct 24, 2023, 14:01

resolved all threads

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 15:46

Commented on CONTRIBUTING.md line 3

Why environmental systems? One could use rtc-tools for other applications, right?

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 15:49

Commented on CONTRIBUTING.md line 12

Should we add a link to the Sphinx webpage here?

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 15:54

Commented on CONTRIBUTING.md line 23

It's also good to mention the python version and the versions of relevant external packages such as casadi, pymoca, and/or numpy.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:00

Commented on CONTRIBUTING.md line 53

New tests are not so much to ensure that existing functionalities work, but rather to ensure that the introduced changes result in the desired behavior and that new functionalities don't accidently break in the future.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:01

Commented on CONTRIBUTING.md line 54

What do you mean by "the issue at hand"? Maybe omit this last part?

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:04

Commented on CONTRIBUTING.md line 51

Should we create an issue before starting a new branch? Just to avoid that people work on the same thing.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:09

Commented on CONTRIBUTING.md line 11

Sometimes you call it pull request and sometimes merge request. Maybe just call it merge request everywhere?

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:10

Commented on CONTRIBUTING.md line 59

Maybe merge this section with the previous on creating pull requests? It seems to be the same topic.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:13

Commented on CONTRIBUTING.md line 125

You don't need Modelica. It is a language and not an external package.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:14

Commented on CONTRIBUTING.md line 126

I don't think it is necessary to mention external packages here. They are automatically installed when installing rtc-tools.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:17

Commented on CONTRIBUTING.md line 132

This is relevant for users and should be in the README.md. Developers probably need the edible installation.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:19

Commented on CONTRIBUTING.md line 163

We also need to install sphinx-rtd-theme: pip install sphnix-rtd-theme.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:22

Commented on README.md line 4

Again, why just environmental systems?

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:30

Commented on README.md line 6

In the first place I think rtc-tools is a tool for optimizing control problems and for simulating models rather than a tool for creating models. Creating models is mostly done using the Modelica language and using rtc-tools-channel-flow etc.

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:31

Commented on README.md line 6

I also suggest to split this line into multiple lines or even paragraphs.

In GitLab by @Ailbhemit on Oct 24, 2023, 16:33

Commented on README.md line 6

Indeed it should be clear that using modelica is not compulsory

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:33

Commented on README.md line 6

Also, the term "competing goals and objectives" seems to come a bit out of nowhere, since nothing about goals was mentioned yet. Also, goals and objectives are the same thing, right?

In GitLab by @SGeeversAtVortech on Oct 24, 2023, 16:35

Some comments but overall these guidelines look pretty good!

In GitLab by @jarsarasty on Oct 24, 2023, 17:05

Thanks for the feedback, @SGeeversAtVortech! I will make the changes.

In GitLab by @jarsarasty on Nov 14, 2023, 09:27

marked this merge request as draft

In GitLab by @jarsarasty on Nov 15, 2023, 15:05

Commented on CONTRIBUTING.md line 59

I merged part of it, and I cross-referenced the relevant sections.