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

GEODE-10291: Add ubuntu 22.04 (jammy) to CI #968

Open
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

moleske
Copy link
Member

@moleske moleske commented May 10, 2022

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

apt-get -y autoremove && \
apt-get autoclean

RUN pip3 install --upgrade pip && \
Copy link
Contributor

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.

doxygen \
graphviz \
python3-pip \
clang-format-6.0 && \
Copy link
Contributor

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.

Copy link
Member Author

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

Copy link
Contributor

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.

Copy link
Member Author

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

Copy link
Contributor

@gaussianrecurrence gaussianrecurrence left a 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

&& bash ${installer} --skip-license --prefix=/usr/local \
&& rm ${installer}

ARG GEODE_VERSION=1.14.4
Copy link
Contributor

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

Copy link
Member Author

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

@gaussianrecurrence
Copy link
Contributor

Hi @moleske & @pivotal-jbarrett, it seems to me that @Moleke addressed all the comments, so I am not sure is this can be merged already?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants