Skip to content

build: Add Makefile; replace inline knit logic with Makefile target in GitHub workflow#1750

Open
kpavlov wants to merge 2 commits intodevelopfrom
kpavlov/makefile-knit
Open

build: Add Makefile; replace inline knit logic with Makefile target in GitHub workflow#1750
kpavlov wants to merge 2 commits intodevelopfrom
kpavlov/makefile-knit

Conversation

@kpavlov
Copy link
Copy Markdown
Contributor

@kpavlov kpavlov commented Mar 26, 2026

Add Makefile; replace inline knit logic with Makefile target in GitHub workflow

  • Add a Makefile for easier local build reproduction. These shortcuts encode the correct flags and task ordering so you don't have to remember them.
  • Replace inline knit logic with Makefile target in GitHub workflow
  • Update CONTRIBUTING.md

@kpavlov kpavlov requested a review from Amaneusz March 26, 2026 08:58
…n GitHub workflow

- Add a Makefile for easier local build reproduction
- Replace inline knit logic with Makefile target in GitHub workflow
- Update CONTRIBUTING.md
@kpavlov kpavlov force-pushed the kpavlov/makefile-knit branch from 18d07cf to 22fd946 Compare March 26, 2026 09:03
@kpavlov kpavlov added the build Build process and CI improvement 🏗️ label Mar 26, 2026
@kpavlov kpavlov marked this pull request as ready for review March 26, 2026 09:08
Copy link
Copy Markdown
Collaborator

@EugeneTheDev EugeneTheDev left a comment

Choose a reason for hiding this comment

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

Sorry, I don't see how this makes things easier. Even in the current setup you can check the docs with a single Gradle command: ./gradlew :docs:knitAssemble. It cleans previous code snippets, generates new ones, and then builds them

@kpavlov
Copy link
Copy Markdown
Contributor Author

kpavlov commented Mar 26, 2026

Sorry, I don't see how this makes things easier. Even in the current setup you can check the docs with a single Gradle command: ./gradlew :docs:knitAssemble. It cleans previous code snippets, generates new ones, and then builds them

@EugeneTheDev, thank you for pointing this out. While ./gradlew :docs:knitAssemble is a concise command, it doesn’t appear to be actively used in the current workflow. Introducing this and other shortcuts into common practices could certainly enhance efficiency and user convenience.

Additionally, is the file counting logic in the current workflow beneficial, or should it be removed?

@Amaneusz
Copy link
Copy Markdown
Collaborator

Amaneusz commented Mar 26, 2026

I generally like the idea of the Makefile as it can simplify things.

  1. when we need to compose gradle command made of many exclusions, flags etc
.PHONY: test
test:
	./gradlew checkLegacyAbi jvmTest :integration-tests:jvmTestClasses --exclude-task :integration-tests:jvmTest
  1. when operating just on gradle tasks does not cover the whole story:
.PHONY: publish
publish:
	rm -rf ~/.m2/repository/ai/koog
	./gradlew publishToMavenLocal


.PHONY: build-compose-example
build-compose-example: publish
	cd examples/demo-compose-app && \
	./gradlew assemble

That said, knit is a weak example — ./gradlew :docs:knitAssemble already does clean + generate + assemble in one shot. That target could be simplified or dropped.

Someone could make a valid argument that this adds something to maintain — but then again, these shortcuts are opt-in and could simplify some developers' workflow, so I'd vote for adding it.

Copy link
Copy Markdown
Collaborator

@Amaneusz Amaneusz left a comment

Choose a reason for hiding this comment

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

I'm ok with adding this if we’ll reach consensus with other devs.
I personally see myself using Makefile but at the same time I did not have that much issue with "gradle only" approach.

If we proceed with it, we should probably change the knit target and simplify it as Andrey suggested

@Rizzen Rizzen requested a review from Ololoshechkin March 26, 2026 10:46
Copy link
Copy Markdown
Member

@Rizzen Rizzen left a comment

Choose a reason for hiding this comment

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

I’m not convinced make is the right abstraction here. This change adds a Makefile and supporting docs, but most of the targets just wrap simple one-liners. That gives very little benefit while introducing extra indirection and another maintenance surface.

It also brings in a Unix-centric tool requirement. make isn’t equally natural or available on Windows (it typically has to be installed separately), which could reduce accessibility rather than improve local reproducibility.

Given how simple these commands are, it seems clearer and more maintainable to keep them as one-liners in CONTRIBUTING.md instead of introducing make.

- Replace verbose knit generation logic with streamlined `knitAssemble` task.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

build Build process and CI improvement 🏗️

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants