Skip to content

chore: Document options naming and remediate#3714

Merged
jamtur01 merged 13 commits intomasterfrom
option_naming
Oct 2, 2020
Merged

chore: Document options naming and remediate#3714
jamtur01 merged 13 commits intomasterfrom
option_naming

Conversation

@jamtur01
Copy link
Copy Markdown
Contributor

@jamtur01 jamtur01 commented Sep 4, 2020

Example changes in this PR:

  • Fingerprinting becomes Fingerprint.
  • The fingerprint_bytes option becomes bytes.
  • The merge_fields option becomes fields.

Signed-off-by: James Turnbull <james@lovedthanlost.net>
The `fingerprint_bytes` options becomes `bytes`.

Signed-off-by: James Turnbull <james@lovedthanlost.net>
Signed-off-by: James Turnbull <james@lovedthanlost.net>
Signed-off-by: James Turnbull <james@lovedthanlost.net>
Signed-off-by: James Turnbull <james@lovedthanlost.net>
Signed-off-by: James Turnbull <james@lovedthanlost.net>
Signed-off-by: James Turnbull <james@lovedthanlost.net>
Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01 jamtur01 marked this pull request as ready for review September 18, 2020 17:44
Comment thread config/vector.spec.toml Outdated
Signed-off-by: James Turnbull <james@lovedthanlost.net>
Copy link
Copy Markdown
Contributor

@MOZGIII MOZGIII left a comment

Choose a reason for hiding this comment

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

It's really nice to have some guidelines on naming!

@binarylogic
Copy link
Copy Markdown
Contributor

The name changes look great, I would just like to think about backward compatibility. I assume it would be easy to map the old names to the new names?

Signed-off-by: James Turnbull <james@lovedthanlost.net>
@jamtur01
Copy link
Copy Markdown
Contributor Author

@binarylogic I've added aliases to the fingerprinting and merge_fields fields but I am unsure who to exactly handle deprecation here. @lukesteensen Do you have input?

@jamtur01 jamtur01 requested a review from lukesteensen October 2, 2020 15:39
Copy link
Copy Markdown
Member

@lukesteensen lukesteensen left a comment

Choose a reason for hiding this comment

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

You've done the aliasing correctly, but we don't necessarily have a good way to surface the deprecation to the user right now. That'd require something a bit fancier to plug into deserialization. I'll make a note to consider this in the config RFC.

@jamtur01
Copy link
Copy Markdown
Contributor Author

jamtur01 commented Oct 2, 2020

Thanks!

@jamtur01 jamtur01 merged commit 2ae986e into master Oct 2, 2020
@jamtur01 jamtur01 deleted the option_naming branch October 2, 2020 22:41
Comment thread src/sources/file.rs
mengesb pushed a commit to jacobbraaten/vector that referenced this pull request Dec 9, 2020
Signed-off-by: Brian Menges <brian.menges@anaplan.com>
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.

5 participants