-
Notifications
You must be signed in to change notification settings - Fork 50
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
GEODE-10291: Add ubuntu 22.04 (jammy) to CI #968
Conversation
docker/ubuntu-22.04/Dockerfile
Outdated
apt-get -y autoremove && \ | ||
apt-get autoclean | ||
|
||
RUN pip3 install --upgrade pip && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can cut out the coveralls bits. We haven't really been using it. Then you can cut out pip above too.
docker/ubuntu-22.04/Dockerfile
Outdated
doxygen \ | ||
graphviz \ | ||
python3-pip \ | ||
clang-format-6.0 && \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gosh, this should probably be clang-format-11.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lol. I just copied pasted the ubuntu-20.04 dockerfile, so I didn't actually look at anything beyond the from line. I'll clean these up.
Also I don't think these are used in ci after another look, they are only here for convenience. Not sure if anyone cares, just an observation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not used by CI. I occasionally use them locally to compile under the alternative platforms when trying to debug a platform specific issue. They get very little love.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
per some outside of here conversation with @pdxcodemonkey it is clang-format-12. There's a lot of inconsistency in the other dockerfiles, and clang-format-12 is not in the default repository for ubuntu 16.04 and 18.04, so will only fix it for ubuntu 22.04 for now and pr the other dockerfiles later
Authored-by: M. Oleske <[email protected]>
Authored-by: M. Oleske <[email protected]>
Authored-by: M. Oleske <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good, except for the version thing
docker/ubuntu-22.04/Dockerfile
Outdated
&& bash ${installer} --skip-license --prefix=/usr/local \ | ||
&& rm ${installer} | ||
|
||
ARG GEODE_VERSION=1.14.4 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering latest version is 1.15.0, maybe this needs to be updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah joys of forgetting about prs for months on end, can bump it
I don't have ami credentials nor concourse credentials to test any of this, so I will need some help if this should be wanted in CI