Skip to content

docs: update Transform examples#415

Merged
lessthanjacob merged 2 commits intoprocore-oss:mainfrom
SaxtonDrey:doc-update-readme-transform-section
Jun 20, 2024
Merged

docs: update Transform examples#415
lessthanjacob merged 2 commits intoprocore-oss:mainfrom
SaxtonDrey:doc-update-readme-transform-section

Conversation

@SaxtonDrey
Copy link
Copy Markdown
Contributor

@SaxtonDrey SaxtonDrey commented May 25, 2024

While learning how to use Blueprinter, I encountered some confusion with the current code examples.
The concepts are well-explained, but there appears to be a discrepancy with the method names used in the examples.

Checklist:

  • I have updated the necessary documentation
  • I have signed off all my commits as required by DCO
  • My build is green

@SaxtonDrey SaxtonDrey requested review from a team and ritikesh as code owners May 25, 2024 07:54
Comment thread README.md
Copy link
Copy Markdown
Contributor

@lessthanjacob lessthanjacob left a comment

Choose a reason for hiding this comment

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

Thanks for opening this!

Reading through these docs again, I think this example is unnecessarily confusing generally. We could remove the custom_columns method entirely, and rework dynamic_fields to make this a bit easier to follow (i.e. more explicit). Something like:

class User
  def dynamic_fields
    case role
    when :admin
      { 
        employer: employer,
        time_in_role: determine_time_in role
      }
    when :maintainer
      { 
        label: label,
        settings: generate_settings_hash
      }
    when :read_only
      { 
        last_login_at: last_login_at
      }
    end
  end
end

@SaxtonDrey SaxtonDrey force-pushed the doc-update-readme-transform-section branch from cc88164 to 77ecb12 Compare June 18, 2024 02:42
Signed-off-by: SaxtonDrey <4phantomron@gmail.com>
Signed-off-by: saxtondrey <4phantomron@gmail.com>
@SaxtonDrey SaxtonDrey force-pushed the doc-update-readme-transform-section branch from 818640c to 3caf726 Compare June 18, 2024 02:47
@SaxtonDrey
Copy link
Copy Markdown
Contributor Author

Thanks for your review and suggestion!
I agree that the current example is too complicated, so I updated PR(I just only took in your suggestion 😄 ).
Let me know if there are any further adjustments needed.

@lessthanjacob lessthanjacob mentioned this pull request Jun 20, 2024
3 tasks
@lessthanjacob lessthanjacob added the documentation Documentation only changes label Jun 20, 2024
@lessthanjacob lessthanjacob merged commit 8a94b25 into procore-oss:main Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Documentation only changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants