Skip to content

Conversation

@maopeixia
Copy link
Contributor

Copy link
Member

@joelmartinez joelmartinez left a comment

Choose a reason for hiding this comment

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

Offhand, this looks fine ... however two things:

  1. I'd like to see either a unit test, or integration test that tests this scenario ... so there would be existing content, and then this would be removed for the new content to be added by the importer. Please feel free to refactor this particular section of the code out into maybe a method in DocUtils that can more easily be unit tested.
  2. Take a look at the linked item in the comments of the devops item ... it's a similar issue with related tags that need to be similarly cleared as what you've done here.

With those changes this would be ready to go :)

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.

2 participants