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.

Sign up for GitHub

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

Jump to bottom

Adjust rendering property #73

Merged
merged 6 commits into from
Oct 3, 2022
Merged

Adjust rendering property #73

merged 6 commits into from
Oct 3, 2022

Conversation

kirkkwang
Copy link
Contributor

A proposed change

Currently if conditions are not met, #populate_rendering returns an empty array which makes the rendering appear in the manifest as such. I propose to remove rendering from the manifest if it is empty so it can be cleaner.

  • add guard for manifest.rendering
  • refactor #populate_rendering
  • disable rubocop PerceivedComplexity
  • adjust specs

@kirkkwang
Copy link
Contributor Author

kirkkwang commented Oct 3, 2022 •
edited
Loading

@cjcolvar
Apparently, be_false and be_true have changed to be_falsey and be_truthy 🤷

I used be false since it was used elsewhere already, let me know if you want it as be_falsey

Anyway, thanks for the suggestion, tests pass on this now!

@cjcolvar cjcolvar force-pushed the adjust-rendering-property branch from 7a2b1c3 to 34c9255 Compare October 3, 2022 17:49
@cjcolvar cjcolvar merged commit b687a54 into main Oct 3, 2022
@cjcolvar cjcolvar deleted the adjust-rendering-property branch October 3, 2022 17:51
@cjcolvar cjcolvar mentioned this pull request Oct 10, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cjcolvar cjcolvar cjcolvar approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@kirkkwang @cjcolvar