SUSE / technical-reference-documentation

SUSE Technical Reference Documentation
https://documentation.suse.com/trd-supported.html
7 stars 22 forks source link

kr rc co branding #72

Closed bwgartner closed 1 year ago

bwgartner commented 1 year ago

Based upon the logo (docinfo) changes, per html/pdf output formats, the PR has updated all the k(ubernetes) r(eference) RC documents to include co-branding logos for the respective IHV partners. It also provides some templates (courtesy of Terry) and the main partner logs in the common space.

After review and acceptance, this content can be merged and documents re-published.

bwgartner commented 1 year ago

@bwgartner Looks good. +1 Thank you! Some smaller comments (you might be already aware):

  • There is a conflict, probably from the changes that Frank pushed. You maybe need to rebase your branch.

Curious what might need to be done in this space

  • Maybe separate the templates from the real logos? You can just create a templates folder. This would prevent daps from trying to include or process the template images. If they aren't included anywhere, why process them? IMHO, they aren't needed during building a book.

Moved those templates (since they are not referenced by any docs, just to be easy to find/use as a starting point) to another directory (./common/images/src/template-svg) per 016c744. Feel free to propose another space as well

tomschr commented 1 year ago
  • There is a conflict, probably from the changes that Frank pushed. You maybe need to rebase your branch.

Curious what might need to be done in this space

The conflict is due to the fileref attribute. You have this line.

<imagedata fileref="{logo}" width="5em" align="center" valign="bottom"/>

whereas main still has the reference to suse.svg:

<imagedata fileref="suse.svg" width="5em" align="center" valign="bottom"/>

As the other attributes are correct, you just need to accept your changes to resolve the conflict.

bwgartner commented 1 year ago
  • There is a conflict, probably from the changes that Frank pushed. You maybe need to rebase your branch.

Curious what might need to be done in this space

The conflict is due to the fileref attribute. You have this line.

<imagedata fileref="{logo}" width="5em" align="center" valign="bottom"/>

whereas main still has the reference to suse.svg:


<imagedata fileref="suse.svg" width="5em" align="center" valign="bottom"/>

Some (3 docinfo files, GS, and a couple of generic templates) still refer to suse.svg, however the kubernetes reference RI/RC in this PR leverage the respective variable (from it's adoc/SA_vars.adoc) for the right :logo: value (default to suse.svg, unless an RC that co-brands with a partner) to utilize.



As the other attributes are correct, you just need to accept your changes to resolve the conflict.
chabowski commented 1 year ago

@bwgartner seems that Tom has still change requests? @tlssuse seems that you should review, too :-).

bwgartner commented 1 year ago

@bwgartner seems that Tom has still change requests? @tlssuse seems that you should review, too :-).

Pretty sure I've taken care of all of Tom's change requests

tlssuse commented 1 year ago

With the latest changes, this looks good to me.

chabowski commented 1 year ago

@fsundermeyer Hi Frank, there is still a conflict showing in the kubernetes docinfo file - would you mind having a look? Merging is not possible yet. Would be great if we could have fixed the issue. Thank you very much!

tlssuse commented 1 year ago

It appears that the remaining conflict was addressed verbally by @bwgartner in this comment.
If I read it correctly, Bryan wants to retain use of a variable for specifying the cover logo; that is, `

`
chabowski commented 1 year ago

Hi @tlssuse - yes I read and understood the comment :-) . But I had a look at the files, and I do not know how to resolve the existing conflict - without resolution, the PR cannot be merged. Hoping that @fsundermeyer knows how to address that logo/image issue.