Skip to content

Use slash instead of dot to separate namespace#128

Draft
cjea wants to merge 2 commits intomasterfrom
ISS-126/generated-topic-names
Draft

Use slash instead of dot to separate namespace#128
cjea wants to merge 2 commits intomasterfrom
ISS-126/generated-topic-names

Conversation

@cjea
Copy link
Contributor

@cjea cjea commented Oct 1, 2020

Issue: #126

Use / instead of . to separate the namespace from the rest of the topic name.

Open questions:

  • Does / count as a valid topic name in terraform?
    • Yes.
~ name  = "rldb-customer-creation" -> "a/b/rldb-customer-creation" # forces replacement

EDIT: terraform plan doesn't complain about the DOT (.) separators either.

~ name  = "rldb-customer-creation" -> "a.b/rldb-customer-creation" # forces replacement

Maybe the above line does not guarantee that SLASH (/) is valid.

  • Was there a reason to transform a namespace from name/space into name-space in the previous code?
    • Seems like the answer is "no". It's just how the kebabCase/snakeCase function worked.
  • Is this rename going to mess up anyone who already generated topic names?
    • Could be. It recreates topics when it renames them.
  • How do I bump reslang version?

@cjea cjea added the change/standard Trivial / minor changes that are low-impact, low risk label Oct 1, 2020
@njaczko
Copy link
Contributor

njaczko commented Oct 1, 2020

Just a drive-by, haven't considered your other questions!

This is where you update the Reslang version: https://github.com/LiveRamp/reslang/blob/master/src/main.ts#L21

@sonarqubecloud
Copy link

sonarqubecloud bot commented Oct 2, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@cjea
Copy link
Contributor Author

cjea commented Oct 2, 2020

Since this introduces a breaking change, it might be best not to merge this until we can get more fixes into the current major version. This puts us in a really tough spot.

@cjea cjea marked this pull request as draft October 2, 2020 16:10
@cjea cjea closed this Oct 6, 2020
@njaczko njaczko reopened this Oct 23, 2020
@ops-github-DU4JOAWE
Copy link

This change is Reviewable

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change/standard Trivial / minor changes that are low-impact, low risk

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants