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

Use slash instead of dot to separate namespace #128

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

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