-
Notifications
You must be signed in to change notification settings - Fork 39
Added information #145
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
base: master
Are you sure you want to change the base?
Added information #145
Conversation
Added some additional information on using the Composer autoloader and /src/ in the Directory structure. Please review my interpretation maybe add something on LTS, will old /inc/ be deprecated in the future?
Created an actual RST table :S and added code examples
Should not be in the pull request.
.vscode/settings.json
Outdated
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.
Forgot to add .vscode to .gitignore obviously xD sorry.
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.
Just a review of the wording/grammar of the new text.
Also, some of this seems like duplicated information from the other recent PR regarding this change:
#139
PRE GLPI 10 | ||
++++++++++ | ||
|
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 entire paragraph must be removed if it's indeed related to versions prior to GLPI v10.
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.
Ping.
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 change currently only adds the label. You want me to drop the paragraph or just accept this change from someone else?
But considering my recent rewrite i think it needs a rewrite for glpi11 as well 😅
In that regard, What are the (structuring) rules regarding documenting for various versions. With GLPI11 alot is changing in regards to the folder structure (dropping marketplace, no more front (if done right), etc)
Should the convention documentation only describe the latest (x) versions, only the supported versions, all versions?
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.
Developers documentation currently only targets GLPI v11.
Changes made in v11 can be marked with a .. versionadded:: 11.0.0
or .. versionchanged:: 11.0.0
and v10 info can be kept - if you want.
But everything older should just be removed (versions are no longer supported).
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.
My previous comments are still relevant.
Co-authored-by: Curtis Conard <[email protected]>
Co-authored-by: Curtis Conard <[email protected]>
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.
See comment
About how to use namespaces and composer autoloader in Plugins.
Maybe add information about future support. Will namespaces be the standard and will /inc be deprecated?
I also think it is wise to add the instruction somewhere to "NOT" use existing GLPI classnames inside the plugin namespace or renaming them using:
use GlpiPlugins\Namespace\ExistingObjectName as SomethingElse;
. I noticed doing so with extends on other existing GLPI objects like CommonDropdown will make the resulting functionality pretty flaky. It should be easy to come up with unique class names.