Skip to content

Conversation

@mupago
Copy link

@mupago mupago commented Dec 10, 2025

Please include answers to these questions as part of your pull request

In the GitHub webUI, use the Write tab to modify the Markdown text that is part of the pull request. For each question simply place an X inside the square brackets, [X], that represents your answer. Make sure there are no blanks inside the brackets, otherwise MarkDown doesn't render properly. Using the Preview tab while editing this form, you can see the formatted/rendered version of the message.

  • Q1: Confirm that you have the right to submit the code that is being contributed. Please consider the origin of your code and confirm you have the appropriate rights to make the submission subject to the Apache 2.0 license that applies to everything in this repository of custom steps. If so, follow the instructions for the Contributor Agreement (which is based on the industry-standard Developer Certificate of Origin (DCO)).
    • Yes, I have the right to submit the contributed code on behalf of myself, my company, or any other owner of the code. I have also attached my signed copy of the DCO to this message.
    • No
  • Q2: Confirm that your contribution does not include any personally identifiable information (PII), for example, in any examples used in your README file.
    • My contribution does NOT include PII data
    • My contribution includes PII data
  • Q3: Confirm your contribution does not include any encryption or other export-controlled technology.
    • My contribution does NOT contain encryption or other export-controlled technology
    • My contribution includes encryption or other export-controlled technology
      github-recovery-codes.txt

Signed-off-by: Murali Pagolu <murali.pagolu@sas.com>
@mupago
Copy link
Author

mupago commented Dec 10, 2025

Initial Commit for HRR - Document Analysis Custom Step [AAIMDEV-5071]

@snlwih
Copy link
Collaborator

snlwih commented Dec 15, 2025

Hi @mupago ,

Thanks for contributing this step. Independent of the offline naming discussion, wanted to provide some feedback here. Primarily related to following best practices around UI labels and cleaning up of user-defined SAS macro variables. ..

  • Make sure the About tab has a change history section and at least lists the current version and release date (which would need to be updated after you have made your changes and update your pull request). It goes without saying that it should be in-sync with the latest release number/date in the readme.md file.
    • Remember, that the only way for a user to know which version of a user-written custom step they are using, is to look at that About tab.
    • An example of best practices approach for the About tab can be found in the _template folder, see _template.step. Not necessarily asking for doc section with links, but the release info definitely needs to be shown.
  • If I'm not mistaken, you create a number of macro variables in your code (call symputx) without deleting them at the end of your step. It is best practice to remove these at the end of your step. See _template.step for example code.
  • You check for the value of the framework generated macro variables representing file/folder UI controls and assume the value representing SAS Content or SAS Server is lower-case. I would use defensive programming and lowcase the value before checking.
  • In many (almost all) labels for UI controls you are currently using a capital for the first character of each word. Best practice for labels is to use sentence casing, just like the out-of-the-box SAS provided steps. You can also find this explicitly mentioned in https://github.com/sassoftware/sas-studio-custom-steps/blob/main/docs/UI-guidelines.md.
  • In your "Additional Options" section you list a number of controls where the label does not end in a question mark. Best practices is to have that label end in a colon.

@snlwih
Copy link
Collaborator

snlwih commented Dec 16, 2025

@mupago , looking at it with fresh eyes this morning I found a some more (small) items that need attention:

  • When using abbreviations that are not commonly known in the IT industry, like SDA or MMR, it is best practice in communication to use the complete term/phrase the first time and mention the abbreviation in round brackets immediately after it. From that moment onwards, you can use the abbreviation to your hearts' content ;-) This applies to both your README.md as well as the UI of the the step itself.

  • No need to have a link back to your readme.md inside the readme.md.

  • As mentioned earlier, please add a changelog section to the About tab. Also add a Pre-requisites section to the About tab that lists Viya version needed and other SAS products needed.

    • Btw. did you know that you we have copy/paste support for UI elements across custom steps? You can have multiple custom steps open in Edit mode in SAS Studio, and then for example in _template.step in the Designer view select a section, right-mouse-button-click and select Copy and then paste it into the Designer view showing your own step. That will maintain all the attributes of the section and all the objects inside that section, including settings for auto-expand and self-explanatory names used in the template for IDs of the UI controls ;-)
  • We also have a best practice in the About tab to have the full name of the step in the first line, followed by a line of equal signs roughly the same length as the first line (because of non-uniform fonts you can't make it show the exact same length, but try to get close), then an empty line, and then "This step ...". Not repeat the name of the step, it is unnecessarily cluttering the info.

    image

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants