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 Debian 10 as supported platform and add missing package for SymEngine #238

Merged
merged 5 commits into from
Jul 6, 2021

Conversation

kinnewig
Copy link
Contributor

@kinnewig kinnewig commented Jul 6, 2021

I added Debian 10 as supported platform and tested if it works.

I also noticed that for the platforms CentOS 7 and Debian 9, the package for SymEngine (i.e. gmp-devel and libgmp-dev respectively) is missing. Therefore, I added these packages.
Furthermore I guess that the package for SymEngine is also missing on other platforms, but I can only test CentOS and Debian at the moment.

@ghost
Copy link

ghost commented Jul 6, 2021

It looks like the two platform files (debian9, debian10) are identical except for the version numbers. Since we just merged the different platforms for Fedora and Ubuntu, we could do the same for Debian. @koecher What do you think?

@Sebastian-47 If you rename one of the Debian platform files to debian.platform, remove the other one and replace in candi.sh line 633 echo debian${OS_MAJOR_VER} by echo debian, then we would have merged the platform files for Debian as well.

@kinnewig
Copy link
Contributor Author

kinnewig commented Jul 6, 2021

Yes, the two platform files were identical. Therefore, I have unified both files as you suggested.

@@ -1,4 +1,4 @@
# debian 9
# debian
Copy link

Choose a reason for hiding this comment

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

debian Debian

#
# Then reboot and run candi again.
##

# on debian 9 the candi installed parmetis 4.0.3 is not recognized correctly
# on debian the candi installed parmetis 4.0.3 is not recognized correctly
Copy link

Choose a reason for hiding this comment

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

on debian On Debian

@ghost
Copy link

ghost commented Jul 6, 2021

Except the comments, it looks reasonable. However, @koecher or @tjhei needs to review it.

#
# Then reboot and run candi again.
##

# on debian 9 the candi installed parmetis 4.0.3 is not recognized correctly
# on Debian the candi installed parmetis 4.0.3 is not recognized correctly
Copy link

Choose a reason for hiding this comment

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

On (capitalized beginning of sentence ... sorry)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I just missed that.

@ghost
Copy link

ghost commented Jul 6, 2021

@Sebastian-47 Maybe you also want to squash your commits into one single commit. You can do this for example with:

git checkout master
git checkout -b debian_platform
git push -u origin debian_platform

git checkout e0e101 (these are Zeros)
git branch -D master
git checkout -b master
git push -u origin master --force

git checkout debian_platform
git rebase -i master

(In this textfile replace all pick by f except the first one)
Ctrl+X
Y
(In case of merge conflicts you need to resolve them and continue the rebase process)

git push --force

Maybe you are more familiar with git and dont need this. In any case you also want to do a backup of the updated files during theses steps ;)

@koecher
Copy link
Contributor

koecher commented Jul 6, 2021

@Sebastian-47 Maybe you also want to squash your commits into one single commit. You can do this for example with:

Squashing can be done as merging strategy in github itself (currently). So there is no need for it here, but thanks for the remark.

@koecher
Copy link
Contributor

koecher commented Jul 6, 2021

@Sebastian-47 Thank you for your contribution. Just a general remark: it would be great if you can create different pull requests if you have different developments, e.g. here the symengine / centos and the debian platform.

@koecher
Copy link
Contributor

koecher commented Jul 6, 2021

@Sebastian-47 Another remark: it would be good if you branch (git checkout master and then git checkout -b new-feature) and do the pull request from your new branch.

@koecher koecher merged commit 7ab5a19 into dealii:master Jul 6, 2021
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.

2 participants