-
Notifications
You must be signed in to change notification settings - Fork 51
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
feat(24.04): libgdiplus and its dependencies #314
base: ubuntu-24.04
Are you sure you want to change the base?
Conversation
Diff of dependencies: slices/fontconfig-config.yaml@@ -1,8 +1 @@
-fonts-croscore
fonts-dejavu-core
-fonts-freefont-otf
-fonts-freefont-ttf
-fonts-liberation
-fonts-noto-core
-fonts-texgyre
-fonts-urw-base35 slices/fonts-urw-base35.yaml@@ -1 +0,0 @@
-xfonts-utils slices/xfonts-encodings.yaml@@ -1 +0,0 @@
-x11-common slices/xfonts-utils.yaml@@ -1,6 +1,5 @@
libc6
libfontenc1
libfreetype6
-x11-common
xfonts-encodings
zlib1g |
88244eb
to
29d5517
Compare
@cjdcordeiro @rebornplusplus can someone approve the workflow please? |
It's running. Must be a first time contributor thing that the workflows do not run automatically. Needs to looked into. |
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.
tyvm @thecoldwine ! This is a big one :)
Could you please:
- add the tests for
xdg-user-dirs
,xfonts-utils
andfontconfig
- comment of the dependency diff (especially the missing x11-common deps)
@cjdcordeiro I did push the deps update (I think), so it would be nice if we could run diff again. On the rest of comments I will redesign slices accordingly. |
@cjdcordeiro I pushed the changes, please have a look. Not sure who should resolve the conflicts. |
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.
Thanks @thecoldwine
The conflicts are in the files you're proposing. I suggest you do a rebase (cause some relevant changes have entered 24.04 since you opened your PR) and then you can fix each conflict individually.
P.S. the diff
comment is updated on every commit.
c83a58d
to
d7f1d5b
Compare
Had to force push due to rebase. |
Co-authored-by: Cristovao Cordeiro <cristovao.cordeiro@canonical.com>
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.
LGTM. Thanks for the changes and tests!
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.
Amazing stuff! It's quite a long PR, but very good job slicing those packages. I only have a few comments below.
essential: | ||
- fonts-noto-mono_fonts |
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.
Shouldn't the fonts-noto-core_config
slice be listed as a dependency here?
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.
Any thoughts?
(Was this previously requested to be secluded?)
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.
@rebornplusplus not really — I can make a slice which will contain both of them, but in general @cjdcordeiro asked me to split packages to fonts and configs which do not depend on each other.
@rebornplusplus @cjdcordeiro please let me know if I need to address the changes in the PR or you will resolve them within the team. It feels like a full circle now, so I will wait for your signal for the action :D |
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.
Hiya, I marked most of my comments resolved per your response. There are still a few I would like your thoughts on.
Additionally, it seems like there are a few conflicts because the upstream has similar changes already. Could you please merge the upstream/main
into this branch?
Thank you.
essential: | ||
- fonts-noto-mono_fonts |
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.
Any thoughts?
(Was this previously requested to be secluded?)
Sorry about the back and forth. I marked a few comments above for you to take another look at. Those are very trivial. Thus, once those are addressed, I can approve this and merge since Cris has approved it already. Thanks. :) |
9160b04
to
b77fb71
Compare
Co-authored-by: Rafid Bin Mostofa <rafid.mostofa@canonical.com>
@rebornplusplus I updated the small changes, primary question is dependency in the font config, but it had been split intentionally after a review. |
@rebornplusplus ping :) |
Proposed changes
This PR adds libgdiplus slice as one of the deps for Mono System.Drawing package.
Following packages had been sliced for this:
File system emitted on the chiseling
Related issues/PRs
#307
Checklist
Additional Context