Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MACF-7: Add generic action so that custom flows can be seen and edited #142
base: master
Are you sure you want to change the base?
MACF-7: Add generic action so that custom flows can be seen and edited #142
Changes from 12 commits
30394e4
7f890bf
f4ae0ce
4bb0d18
3c8ed90
3b7cb37
2aa0415
b12eed9
e88e32c
41f91ae
411b364
bc5d704
085748e
9d2997a
02b109c
fa6943c
b13542c
bbb9af9
70e074e
f54cec9
3b97d35
d125a08
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
How about abstracting this behavior into a local function, taking as parameter
$template
? That function can then be named after its behavior, negating the need of describing what it does as a comment (as its name would indicate that).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.
This would be better as part of the description of
app#miscGetSchem
, not at the call site. The reason is, what if this methods gets called somewhere else?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.
Again, the name of the method already describe its purpose here, not sure what this comment tries to clarify? Any more in-depth explaining should be moved to the method's description.
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.
Same here, this behavior can be abstracted into a function if not straighforward enough that it needs a comment.
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.
If you check the structure if this file, you will notice that API helpers (methods wrapping direct API calls) are located at the end of the file (by convention).
Please try to keep it that way and move your last two methods above that (anywhere before
/* API helpers */
should be good).