Skip to content

Conversation

nunomaduro
Copy link

Q A
Bug fix? no
New feature? yes
BC breaks? no
Need Doc update yes

Describe your change

Adds methods replace_all_rules and replace_all_synonims.

@julienbourdeau julienbourdeau changed the title Adds replace_all_rules and replace_all_synonims Adds replace_all_rules and replace_all_synonyms Nov 20, 2018
Copy link

@julienbourdeau julienbourdeau left a comment

Choose a reason for hiding this comment

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

Great PR 👍
I commented on Synonyms, but the same apply to rules

@param synonyms the synonyms to upload as a list of python dictionary.
the dictionary must contain an objectID key.
"""
for synonym in synonyms:

Choose a reason for hiding this comment

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

Should we really add this verification in v1's 🤔
This makes replacesAllSynonyms and batchSynonyms behave differently.

Copy link
Author

Choose a reason for hiding this comment

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

@julienbourdeau I believe new methods should follow the specs. The specs defines that synonyms must contain a valid ObjectID. I think it's fine.

'replaceExistingSynonyms': True
}

return self._req(False, '/synonyms/batch', 'POST', request_options, params, data=synonyms)

Choose a reason for hiding this comment

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

Did you not use batch_synonyms on purpose because of the requestOptions?

Copy link
Author

Choose a reason for hiding this comment

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

@julienbourdeau Because all the extra works of setting the params of the batch_synonyms. Is just more simple call one line.

@nunomaduro nunomaduro force-pushed the feat/adds-replace-rules-and-replace-synonims branch from 81d04e0 to 4d471da Compare November 23, 2018 13:49
@nunomaduro nunomaduro merged commit 4d471da into master Nov 23, 2018
shortcuts pushed a commit that referenced this pull request Nov 24, 2023
* chore(ci): use different branch to test release

* chore: process release on main_test branch

* chore: revert release.config.json
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