Opened 2 years ago

Closed 21 months ago

#4233 closed enhancement (complete)

Add new concat string function to classification expression

Reported by: fdupont Owned by: fdupont
Priority: medium Milestone: Kea1.1
Component: classification Version: git
Keywords: classification Cc:
CVSS Scoring: Parent Tickets:
Sensitive: no Defect Severity: N/A
Sub-Project: DHCP Feature Depending on Ticket:
Estimated Difficulty: 0 Add Hours to Ticket: 0
Total Hours: 0 Internal?: no

Description

concat(x,y) (symmetrical to substring).

Subtickets

Change History (19)

comment:1 Changed 2 years ago by fdupont

  • Owner set to fdupont
  • Status changed from new to accepted

comment:2 Changed 2 years ago by fdupont

  • Owner changed from fdupont to UnAssigned
  • Status changed from accepted to reviewing

Done. Stacked on #4232 (and indirectly on #4233).

comment:3 Changed 2 years ago by marcin

Are you doing it based on a design or something? We normally put a requirements document, design, review and implementation is a very last thing we do. So, what is your work based on?

comment:4 follow-up: Changed 2 years ago by fdupont

On requirements and design (more accurately this will be in the documentations when they will be updated post 1.0). Note at some possible exceptions in #4231 these extensions are clearly for 1.1 (and they include a sub-option selector which requires a short discussion about its parameter: code only or space name + <separator> + sub-option name alternative too).

comment:5 in reply to: ↑ 4 ; follow-up: Changed 2 years ago by marcin

Replying to fdupont:

On requirements and design (more accurately this will be in the documentations when they will be updated post 1.0). Note at some possible exceptions in #4231 these extensions are clearly for 1.1 (and they include a sub-option selector which requires a short discussion about its parameter: code only or space name + <separator> + sub-option name alternative too).

I am not asking whether this is going to be in the documentation in 1.1. This is obvious that new functionality would have to be in the documentation.

I am asking if what you're doing have been designed and reviewed as part of the 1.1 effort?

comment:6 in reply to: ↑ 5 ; follow-up: Changed 2 years ago by fdupont

Replying to marcin:

I am asking if what you're doing have been designed and reviewed as part of the 1.1 effort?

=> was the 1.1 effort really started?

comment:7 in reply to: ↑ 6 Changed 2 years ago by marcin

Replying to fdupont:

Replying to marcin:

I am asking if what you're doing have been designed and reviewed as part of the 1.1 effort?

=> was the 1.1 effort really started?

No. So, this seems to be odd that you're working on this ticket. But, if you are, where is the design? I am confused.

comment:8 Changed 22 months ago by tomek

  • Milestone changed from Kea-proposed to Kea1.1

That's within the scope of the requirements and the [wiki:ClientClassificationDesign design, moving to 1.1.

comment:9 Changed 22 months ago by tomek

  • Component changed from Unclassified to classification

comment:10 Changed 22 months ago by sar

  • Owner changed from UnAssigned to sar

comment:11 follow-up: Changed 22 months ago by sar

  • Owner changed from sar to fdupont

The copyrights should be updated to include this year.

It needs a change log entry and the docs need to be updated to include the concat function (kea/doc/guide/classify.xml)

The code looks fine.

comment:12 in reply to: ↑ 11 ; follow-up: Changed 22 months ago by fdupont

Replying to sar:

The copyrights should be updated to include this year.

=> the code was written last year but I agree the copyrights must be upgraded anyway. Done.

It needs a change log entry and the docs need to be updated to include the concat function (kea/doc/guide/classify.xml)

=> I am waiting for #4257 to be sure we want a function and not an operator (e.g., + or ^). I believe for the change log we only need something trivial like 'Added a concat function in classification which concatenates two strings. (Trac #4233, git ...)'

The code looks fine.

=> so it could be merged as soon as higher order stuff is ready?
-

comment:13 Changed 22 months ago by fdupont

  • Owner changed from fdupont to UnAssigned

comment:14 in reply to: ↑ 12 ; follow-up: Changed 22 months ago by sar

  • Owner changed from UnAssigned to fdupont

Replying to fdupont:

Replying to sar:

The copyrights should be updated to include this year.

=> the code was written last year but I agree the copyrights must be upgraded anyway. Done.

I don't see any recent changes, have you pushed your changes?

It needs a change log entry and the docs need to be updated to include the concat function (kea/doc/guide/classify.xml)

=> I am waiting for #4257 to be sure we want a function and not an operator (e.g., + or ^). I believe for the change log we only need something trivial like 'Added a concat function in classification which concatenates two strings. (Trac #4233, git ...)'

I think that would be sufficient

The code looks fine.

=> so it could be merged as soon as higher order stuff is ready?
-

Assuming there aren't more changes to the code yes.

comment:15 Changed 22 months ago by sar

We discussed function vs operator and decided to go with a function format: concat(a, b)

comment:16 in reply to: ↑ 14 Changed 22 months ago by fdupont

Replying to sar:

Replying to fdupont:

Replying to sar:

The copyrights should be updated to include this year.

=> the code was written last year but I agree the copyrights must be upgraded anyway. Done.

I don't see any recent changes, have you pushed your changes?

=> I pushed them but not on the trac4233 branch. Fixed now.

comment:17 Changed 21 months ago by fdupont

  • Owner changed from fdupont to UnAssigned

Added Concat in the doc. Ready to be merged?

comment:18 Changed 21 months ago by sar

  • Owner changed from UnAssigned to fdupont

It looks fine to me

comment:19 Changed 21 months ago by fdupont

  • Resolution set to complete
  • Status changed from reviewing to closed

Merged (manually as the branch contained unreviewed ipaddress support).

Note: See TracTickets for help on using tickets.