Skip to content
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

Update shadermodel.md #1843

Merged
merged 1 commit into from
Apr 15, 2023
Merged

Update shadermodel.md #1843

merged 1 commit into from
Apr 15, 2023

Conversation

thetyl
Copy link
Contributor

@thetyl thetyl commented Mar 24, 2022

What does this PR do?
Update shadermodel documentation to replace auto-generated text.

Thanks for the contribution! Please provide a concise description of the problem this request solves.

How does this PR change Premake's behavior?
Modifies the Premake documentation.

Are there any breaking changes? Will any existing behavior change?
No.

Anything else we should know?
No.

Add any other context about your changes here.

Did you check all the boxes?

  • [CHECK ] Focus on a single fix or feature; remove any unrelated formatting or code changes
  • [CHECK ] Add unit tests showing fix or feature works; all tests pass
  • [CHECK ] Mention any related issues (put closes #XXXX in comment to auto-close issue when PR is merged)
  • [CHECK ] Follow our coding conventions
  • [CHECK ] Minimize the number of commits
  • [ CHECK] Align documentation to your changes

You can now support Premake on our OpenCollective. Your contributions help us spend more time responding to requests like these!

Copy link
Member

@nickclark2016 nickclark2016 left a comment

Choose a reason for hiding this comment

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

Appreciate you taking time to update the shader docs. Everything is relatively minor, just want to keep things consistent across the docs 👍


```lua
shadermodel (value)
shadermodel "value"
Copy link
Member

Choose a reason for hiding this comment

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

Let's use shadermodel ("value") to keep consistent with other docs - (e.g.) https://premake.github.io/docs/objdir/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make the relevant changes as requested. Hey no problem, I've never been good at documentation but just wanted to make these small fixes to make things a little more clear for the future.

Copy link
Member

Choose a reason for hiding this comment

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

Appreciate all help with docs. When you get those changes out, I'll rereview and merge them so we can get it out.

Copy link
Member

Choose a reason for hiding this comment

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

Just poking this for line 39 as well.

@thetyl thetyl requested a review from nickclark2016 March 29, 2022 23:57
@tommitytom
Copy link

Is there an example of this shader stuff anywhere? And does it only support HLSL?

@thetyl
Copy link
Contributor Author

thetyl commented Aug 4, 2022

Yes it only supports HLSL. There is an example usage included in this change.

@thetyl
Copy link
Contributor Author

thetyl commented Aug 4, 2022

@nickclark2016 Could you please approve this PR?

@nickclark2016
Copy link
Member

Just saw the message, the last several weeks have been hectic for me. Still waiting on you to address that last comment, then I'll go ahead and merge.

@KyrietS
Copy link
Member

KyrietS commented Nov 28, 2022

@TylerDahl ?

Changed shadermodel parameter format.

Changed example syntax.
@thetyl
Copy link
Contributor Author

thetyl commented Nov 28, 2022

@KyrietS Addressed the final issue for line 39.

@KyrietS
Copy link
Member

KyrietS commented Nov 29, 2022

Let's wait for @nickclark2016's approval

@samsinsane samsinsane merged commit edfb1c8 into premake:master Apr 15, 2023
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