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

Case sensitive or not? #35

Closed
Tracked by #78
emiltin opened this issue Oct 30, 2019 · 5 comments
Closed
Tracked by #78

Case sensitive or not? #35

emiltin opened this issue Oct 30, 2019 · 5 comments
Labels
Milestone

Comments

@emiltin
Copy link
Contributor

emiltin commented Oct 30, 2019

Section https://github.com/rsmp-nordic/rsmp_core/blob/master/rst/rsmp.rst#basic-structure states:

Parsing is recommended to be performed case insensitive

I think this is too weak. We should be clear about whether parsing should be case sensitive or insensitive. Errors can arise if systems don't agree.

It's also not clear to me whether this means you're allowed to send commands with different casing. Is 'status', 'Status', and 'STATUS' all allowed? I don't think they should be, I think we should enforce a specific casing.

Unfortunately, casing does not currently seem very consistent, e.g. look at these fields from the example at https://github.com/rsmp-nordic/rsmp_core/blob/master/rst/rsmp.rst#message-structure:

    "mType": "rSMsg",
    "aSp": "Issue",
    "ack": "notAcknowledged",
    "aS": "active",
    "xACId": "Serious lamp error",
    "cat": "D",

It's a rather confusing combination of casing and spacing.

@emiltin
Copy link
Contributor Author

emiltin commented May 10, 2021

flag for 3.1.6?

@otterdahl
Copy link
Contributor

Yes. I think so

@emiltin emiltin added this to the 3.1.6 milestone May 11, 2021
@emiltin
Copy link
Contributor Author

emiltin commented May 11, 2021

If we want to follow semantic versioning, the next version should be called 4.0 if it includes breaking changes.
See rsmp-nordic/rsmp_sxl_traffic_lights#113.

@emiltin emiltin modified the milestones: 3.1.6, 3.2 Sep 16, 2021
@emiltin emiltin added clarify and removed breaking breaking change labels Sep 16, 2021
otterdahl added a commit that referenced this issue Nov 1, 2021
@otterdahl otterdahl mentioned this issue Nov 1, 2021
18 tasks
@otterdahl
Copy link
Contributor

We're changing this to case sensitive.

otterdahl added a commit that referenced this issue Nov 4, 2021
@otterdahl
Copy link
Contributor

I'm closing this. Feel free to open this issue again if you have any feedback on the draft

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants