-
Notifications
You must be signed in to change notification settings - Fork 51
Adding feature for specifying namespace #51
Conversation
… to at most two strings
String namespace = null; | ||
|
||
if (splitByPeriod.length == 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about if the length of the array is greater than or less than 2?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some thoughts:
- What if there is no period(.) ? namespace should be empty ?
- If length is 1(example: ".some_regex" or "something."; I feel we should throw an error because its hard to guess whether user intended to provide the namespace or the regex
- if length > 3 . Error out
@awillia2 @vamshi-sfdc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another approach : we could take in user input and process the input to a format "something1.something2" so that the length is always 2 .
- For no namespace we could use something like "NONAMESPACE" (a restricted keyword that cant be used for namespace) for something1
- For multiple namespaces we can generate multiple namespace: regex pairs
String namespace = null; | ||
String classname = strLine; | ||
|
||
if (splitByPeriod.length == 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if the length is not 2? We need to handle all the cases and consider
a. users who dont use namespace
b. users who use a namespace
c. users who use multiple namespaces
d. users who would want to use a namespace as 'default'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good points adarsh👍
You echo our thoughts. The solution to this Feature request need to be more broader covering all scenarios and posibilities of namespace domain
@adarsh-ramakrishna-sfdc since you spent quality time in industry packeging and namespaces can you add clarification around the requirements itself on #30 and #23
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Salesforce's namespace cannot contain periods .. it's not like a Java package that can be infinitely long
|
||
if (splitByPeriod.length == 2) { | ||
namespace = splitByPeriod[0].trim(); | ||
classname = splitByPeriod[1].trim(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Such ordering must be documented very clearly in usage guide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what if my namespace has more than two periods?
Are we trying to address issue #30 with this PR? |
Rejecting PR to rework on the approach, scope and overall solution |
No description provided.