feat(settings): Add support to tables in toml settings

[danieljmv01]

Correct loop so that it iterates over the values and not the keys of the custom fragment types if provided.

Resolves #361

• Iterate through values if types is a mapping instead of an array.

Resolves: twisted/#361

[adiroiban]

Looks good. Thanks.

Can you add a test for custom types?

In this way we will make sure that this feature will always work and it will not be broken by future work.

But if you don't have time for a test, that is ok.

We can merge it as it is.

Thanks again!

[danieljmv01]

Yes, I think I can do it tomorrow. Thanks!

[adiroiban]

can we these as 2 tests with separate docstring.

I am not sure that is the targeted difference between readme_format and alternative_format.

For the docstring, my suggetion is something like this

Custom fragment categories can be defined inside the toml config file using implicit file extension.
The file extension is the same as directory name.

Custom fragment categories can be defined inside the toml config file using custom file extension.
The file extension is define by the section name.

Does these docstrings make sense?
Is this what it is tested?

You can refactor the shared code in a helper function...but I think that it helps to have 2 separate test functions with 2 docstrings as they are covering 2 separate uses cases.

Thanks!

[danieljmv01]

Done.

[adiroiban]

and since this is a new feature, can you also update the configuration ? Thanks!

[codecov]

Codecov Report

Merging #362 (692e364) into master (0e6a479) will increase coverage by 0.05%.
The diff coverage is 100.00%.

❗ Current head 692e364 differs from pull request most recent head fbc813a. Consider uploading reports for the commit fbc813a to get more accurate results

@@            Coverage Diff             @@
##           master     #362      +/-   ##
==========================================
+ Coverage   96.81%   96.86%   +0.05%     
==========================================
  Files          20       20              
  Lines        1194     1214      +20     
  Branches      108      110       +2     
==========================================
+ Hits         1156     1176      +20     
  Misses         20       20              
  Partials       18       18              

Impacted Files	Coverage Δ
src/towncrier/_settings.py	96.59% <100.00%> (+0.24%)	⬆️
src/towncrier/test/test_settings.py	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0e6a479...fbc813a. Read the comment docs.

[danieljmv01]

and since this is a new feature,

Initially I didn't know that I was passing the configuration in an unexpected way, and I considered this a bug. I tried to just fix it, without adding new features.
But, since this is not a bug, but a new feature, I would like to stop using (or make it optional) the "directory" key if passing tables (it will be mandatory if using a table array).
Also, I would like to make optional including "showcontent" key (e.g., assume that type has "showcontent": True if not passed).
Do you consider that doing this is ok or do you prefer those changes in an additional PR?

can you also update the configuration ? Thanks!

I am not sure about what do you refer. The configuration docs?

Thanks!

[adiroiban]

Is this PR still valid or can be closed as invalid?

My understanding is that you found a way to use the existing towncrier functionality to implement your requirements.

I guess that we ended up with this PR since the existing documentation was not clear enough.

So maybe the documentation can be improved.

[danieljmv01]

Hi @adiroiban I would like to update the PR, I want to update the docs regarding this topic, but the docs sources appears to exists in a branch that is not master.
Is this intended and should I create another PR to that branch or should I take the docs/ dir as it is in the other branch and include it with the changes in this PR?

[adiroiban]

@danieljmv01 . I have created #367 to solve the docs issue.

I prefer to have the code and docs in the same repo.

I agree that things are very confusing with the current setup.

So, if you have time, I am more than happy to review and approve a PR which moves the doc files into the main branch.

[danieljmv01]

Perfect, I will do it today after work.

[adiroiban]

@danieljmv01 can we close this PR? is there any value in having it merged?

Thanks

[danieljmv01]

@adiroiban as is right now: no I don't see any value added.

I wanted to keep changes as minimal as possible, and ended not adding value. What I would like is to give users a simpler, clearer and more intuitive and clear way to configure custom fragment types. I want to support this:

code-block-1:

    [tool.towncrier.type.feat]
    name = "Features"
    showcontent = true

Since the title of each table would describe the custom fragment type (see code-block-1) the field directory is not necessary in this way of configuration.

In the alternative (existing) way to configure custom types you have to include the directory field, that then will be use as key when transforming the array of dicts to a dict.

I would rather wait until #368 is merged to rebase and update this, so I can document this in this PR. After rebasing this branch, I will update the code to support omitting the directory field. If you still consider that it adds no value, then I'm fine with closing this PR.

[adiroiban]

Ok.No hurry from my side.
let me know when this PR is ready from a new review.

[danieljmv01]

@adiroiban I pushed to another branch since this is not a bug.

I have created #370, I will close this one.