StjerneIdioten / I3D-Blender-Addon

Rewriting the I3D blender addon from scratch and adding long-sought community features
GNU General Public License v3.0
66 stars 18 forks source link

feat: warn and assign default material for empty material slots #114

Closed kbrandwijk closed 3 years ago

kbrandwijk commented 3 years ago

As I ran into #97 I decided to put a PR together. I've decided to treat is a missing material, and assign the default material to it, and include a single warning in the log per mesh.

i3dio.node_classes.node.IndexedTriangleSet:populate_from_evaluated_mesh:WARNING: [Cube] triangle(s) found with empty material slot, assigning default material
StjerneIdioten commented 3 years ago

Would it be possible for you to change the commit message to follow AngularJS Commit format ?

I don't enforce it on the repository (yet), but it is the format expected by the build system to generate changelogs and determine build type.

It is my fault for not having it in the developer docs yet.

kbrandwijk commented 3 years ago

I'm fine with changing it, but looking at the guidelines you linked, I'm not sure what you want changed. As far as I can see, I used the correct header (correct type, scope is optional, imperative, present tense). But if anything specific needs to change, please let me know and I'll be happy to.

May I also point you at https://www.conventionalcommits.org/en/v1.0.0-beta.2/ as a more generic and widely adopted version of the AngularJS commit format (although 99% the same). I think that would serve you better in the long run.

And last but not least, I couldn't really figure out which branch to create the PR against. It seemed as though develop was actually behind of master (at least looking at the readme and a few other things), but maybe I was mistaken. Do you want future PRs to be based of develop, and opened against develop?

StjerneIdioten commented 3 years ago

Looking through that link, it is actually exactly what the build tools uses. They just refer to that other page that I linked to for some reason. But the plugin used for the semantic-versioning is called "conventional commits" sooo. And I had just missed the point that scopes are optional, so your message is fine.

And yes, develop is where I would like PR's to go. The reason for develop being behind master is that github shows the readme from master and uses the other github specific files from that branch as well. So I had just edited them via the webinterface, but not pushed it back into develop. Everything code related goes through develop first, unless it is a hotfix for something which has snuck into the master and breaks the exporter in some way. I need to setup/document some more guidelines, when it comes to collaboration ;-)

kbrandwijk commented 3 years ago

Alright cool, I'll make sure to work against develop for future ones. And I've actually worked on the semver plugin you are using in the past, as well as the CLI tool for setting up CI (I'm also a member of the semantic-release organisation), so that's why I'm familiar with the topic. I highly recommend the commitizen CLI tool for all commits.

StjerneIdioten commented 3 years ago

As do I, it is actually one of the only things I have documented in the toolchain section of the i3d exporter documentation.

kbrandwijk commented 3 years ago

Oooh, there's docs 😄

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 2.2.0-dev.1 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket:

StjerneIdioten commented 3 years ago

Yeah, currently it only generates from the master branch though. But I would like to setup different versions for different branches. It is all sphinx docs with the rtd-theme. So very familiar to Blenders own docs.

I even link to the docs at the top of the readme 😅

github-actions[bot] commented 3 years ago

:tada: This PR is included in version 3.0.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: