Skip to content

Doc improvements on 'Custom Builds' #1837

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

Merged
merged 25 commits into from
May 22, 2025
Merged

Doc improvements on 'Custom Builds' #1837

merged 25 commits into from
May 22, 2025

Conversation

daogrady
Copy link
Contributor

@daogrady daogrady commented May 12, 2025

Minor Changes

  • consistent formatting of config values in monospace font (rather than italics, which is used more for file names)
  • some changes in wording
  • make connections within the guide explicit that were implicit before
  • added file name header to snippets to make it more immediately clear where they are supposed to go
  • use <Config> tag where appropriate

Structural Changes From Manual Testing For CDS9

  • 🔴 bad, shows bad practices
  • 🟡 could be improved, best practices are missing
  • 🟢 good (not necessarily perfect)

I added a related snippet for convenient searching of the relevant section, followed by my feedback.


provides options you can use to switch the copy behavior on build task level on or off

🔴 the following example doesn't really make it clear how the behaviour can be changed or what the options shown in the configuration do exactly. ✅


If custom build tasks are configured, those properties have precedence For example, you want to configure the src folder and add the default models. To achieve this, do not define the model option in your build task.

🔴 How else should we do it then? ✅


Such a setup is often referred to as a [monorepo]

The link then leads to an external blog entry about monorepos.

🟡 This feels a bit weird and out of place. There doesn't seem to be an official guide about mono repos, which is probably the reason why this link was used instead. But maybe we can find something else? ✅ (did not find anything more official)


The for property defines the executed build task type. Currently supported types are:

The section then lists various parameters for the for parameter.

🟡 I am not entirely sure how to feel about this section. While it currently seems to be up to date, it will inherently become outdated at some point and is basically a clone of what cds build --help would list under the appropriate section for --for. Could we instead have the output of cds build --help here and make that more explicit instead? We do this similarly for cds-typer.

✅ (postponed: /cds-tools/issues/1541)


It is resolved based on the root folder of your project.

🟡 The following section shows an example of a configuration done in package.json. This is one of multiple places where we can see this pattern. We have the <Config> custom tag to show various ways of including a configuration, aside from package.json nowadays. Could/ should we use this more excessively here? ✅


for each workspace dependency of your project that has a * version identifier

🔴 This needs an example! ✅


A build plugin is a Node.js module complying to the CDS plugin architecture

The following code blocks shows an outdated version of the postgres build plugin.

🔴 I don't think we should duplicate the code at this point. We link to the main branch of the repository containing this plugin just above. I don't see how showing a larger code block that will inherently be outdated without added explanation brings any value to the table. (The section after that does go into detail about some of the methods, but not all. So even if we wanted to use this particular plugin as an illustration of the API, it is just not complete and would benefit more from having the actual API doc included at this point.)
Opportunity to include exported type information inside CAPire?! @chgeo

✅ (we discussed it and came to the conclusion to leave it in. Types can follow at a later point in time.)


@chgeo chgeo changed the title [WIP] Update custom-builds.md [WIP] Doc improvements on 'Custom Builds' May 15, 2025
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Spelling Mistakes

  • guides/deployment/custom-builds.md:109:91 Unknown word "myfolder"

Generally, for each spelling mistake there are 2 ways to fix it:

  1. Fix the spelling mistake and commit it.
  2. The word is incorrectly reported as misspelled → put the word on the project-words.txt list, located in the root project directory.

@daogrady daogrady marked this pull request as ready for review May 21, 2025 07:09
@daogrady daogrady requested a review from renejeglinsky as a code owner May 21, 2025 07:09
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Spelling Mistakes

  • guides/deployment/custom-builds.md:117:91 Unknown word "myfolder"

Generally, for each spelling mistake there are 2 ways to fix it:

  1. Fix the spelling mistake and commit it.
  2. The word is incorrectly reported as misspelled → put the word on the project-words.txt list, located in the root project directory.

@daogrady daogrady requested a review from joergmann May 21, 2025 07:10
@daogrady daogrady changed the title [WIP] Doc improvements on 'Custom Builds' Doc improvements on 'Custom Builds' May 21, 2025
daogrady and others added 3 commits May 21, 2025 14:47
Co-authored-by: Christian Georgi <chgeo@users.noreply.github.com>
Co-authored-by: Christian Georgi <chgeo@users.noreply.github.com>
@swaldmann swaldmann self-requested a review May 21, 2025 14:14
daogrady and others added 2 commits May 22, 2025 07:32
Co-authored-by: René Jeglinsky <rene.jeglinsky@sap.com>
Co-authored-by: René Jeglinsky <rene.jeglinsky@sap.com>
@daogrady daogrady enabled auto-merge May 22, 2025 08:37
@daogrady daogrady added this pull request to the merge queue May 22, 2025
Merged via the queue into main with commit 5804d87 May 22, 2025
4 checks passed
@daogrady daogrady deleted the daogrady-patch-1 branch May 22, 2025 12:38
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.

5 participants