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

Changes to documentation for metacat #472

Merged
merged 24 commits into from
Aug 8, 2024

Conversation

shubham-s-agarwal
Copy link
Collaborator

Adding more description to the config for metacat and adjusting text to avoid rendering issues

Adding more description to the config for metacat and adjusting text to avoid rendering issues
Copy link
Member

@tomolopolis tomolopolis left a comment

Choose a reason for hiding this comment

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

couple of little extras pls

medcat/config_meta_cat.py Show resolved Hide resolved
medcat/config_meta_cat.py Show resolved Hide resolved
Copy link
Member

@tomolopolis tomolopolis left a comment

Choose a reason for hiding this comment

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

couple more things to clarify

medcat/config_meta_cat.py Show resolved Hide resolved
medcat/config_meta_cat.py Show resolved Hide resolved
@shubham-s-agarwal shubham-s-agarwal marked this pull request as draft August 6, 2024 08:51
@shubham-s-agarwal shubham-s-agarwal marked this pull request as ready for review August 6, 2024 11:01
@shubham-s-agarwal
Copy link
Collaborator Author

Fixed the formatting issue, all looks good now finally!

Copy link
Collaborator

@mart-r mart-r left a comment

Choose a reason for hiding this comment

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

On top of my few comments:

  • I think I left a separate comment on disable_component_lock that's not a part of the 'review'
  • There's currently linting issues that need to be addressed for the GHA workflow to be successful.

medcat/config_meta_cat.py Show resolved Hide resolved
medcat/config_meta_cat.py Show resolved Hide resolved
@shubham-s-agarwal shubham-s-agarwal marked this pull request as draft August 7, 2024 08:37
@shubham-s-agarwal shubham-s-agarwal marked this pull request as ready for review August 7, 2024 11:55
Copy link
Collaborator

@mart-r mart-r left a comment

Choose a reason for hiding this comment

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

Looks good to me

Copy link
Member

@tomolopolis tomolopolis left a comment

Choose a reason for hiding this comment

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

lgtm

@shubham-s-agarwal shubham-s-agarwal merged commit 33e32fd into master Aug 8, 2024
8 checks passed
@mart-r
Copy link
Collaborator

mart-r commented Aug 12, 2024

@shubham-s-agarwal Just an FYI for the future, let's try to squash-and-merge when merging in PRs. That way we don't pollute the master branch with a bunch of commits with messages that are only relevant to the PR, not the whole project. By squashing, we only keep the relevant information from the P.

When a PR is ready to merge, there's a drop down for merging. Once you've selected squash and merge once, it should remember it for the next time.

@shubham-s-agarwal
Copy link
Collaborator Author

That makes sense, I'll be sure to do that from next time.
Thanks Mart!

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.

3 participants