Skip to content

Add tool definition for cmake - #504

Merged
dennisdoomen merged 2 commits into
Fallout-build:mainfrom
IT-VBFK:feat/tools/cmake
Jul 19, 2026
Merged

Add tool definition for cmake#504
dennisdoomen merged 2 commits into
Fallout-build:mainfrom
IT-VBFK:feat/tools/cmake

Conversation

@IT-VBFK

Copy link
Copy Markdown
Contributor

@IT-VBFK
IT-VBFK requested a review from a team as a code ownerJuly 17, 2026 17:31
@ITaluoneITaluone added enhancement New feature or request target/vCurrent Targets the current version labels Jul 17, 2026
@dennisdoomen

Copy link
Copy Markdown
Collaborator

Should there not be a .generated.cs file as well?

@IT-VBFK

IT-VBFK commented Jul 18, 2026

Copy link
Copy Markdown
ContributorAuthor

Hmmm... the contribution guidelines stating to explicity not to commit generated tool files (since they are done once before a release).

If this should be different here vs. NUKE this may should be mentioned.

@fredericvaillancourt

Copy link
Copy Markdown

Adding myself to the discussion to get the tool generated answer. Generated my files but it does change the generated from (url) ... several places in every tools ... In fact the current url is from a form and not even the official. Personally I don't like generated files in source control except when they are quite difficult to generate. Waiting to get a definitive answer before I make my PR.

@IT-VBFK

IT-VBFK commented Jul 18, 2026

Copy link
Copy Markdown
ContributorAuthor

I vote for removing them all, because fixing errors in tools wrappers has to go into the json anyway and would be overwritten when not done).

I imagine following workflow

  • add the GenerateToolSettings as additional release pipeline executed target
  • these files (intermediately generated) compiles into the production assetes (dll/nuget package).

You can generate the wrappers locally if you searching/fixing a wrapper bug.

Edit:
Ok.. I found a few tool wrappers who wraps a nuget package. These wrappers use partial classes and reference a generated value in this additional partial class.

@ChrisonSimtian

Copy link
Copy Markdown
Collaborator

look there is a series of things on this topic, but tldr is: commit the *.Generated.cs for now please.
We could probably build those as part of the build but then thats build time on the CI that might as well sit on your machine locally ;-)
Not sure why and how but the convention and agents.md are wrong there. Might be worth to do a follow up and fix that

@ChrisonSimtian

Copy link
Copy Markdown
Collaborator

oh and since this one is a tad grave digging (4 year old PR), lets make sure that the cmake stuff is still up2date please, yes? :-)
I personally dont use cmake so I rely a bit on someone who does to test this and give their opinion

@IT-VBFK

Copy link
Copy Markdown
ContributorAuthor

I already asked AI to add missing parameters and check the rest. Since I don't use cmake either (I'm a 100% .NET guy) we have to rely on actual c/c++ devs I suppose 🫣

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancementNew feature or requesttarget/vCurrentTargets the current version

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@IT-VBFK@dennisdoomen@fredericvaillancourt@ChrisonSimtian@ITaluone