Skip to content
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

Document plugin manifests #25

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

sersorrel
Copy link

substantially based on Franz's messages here: https://discord.com/channels/581875019861328007/860813266468732938/1212577414807429170

adjacent things that aren't included:

  • how to set changelogs (from PR, from manifest.toml, from plugin manifest), incl. nofranz
  • how to set icons (in dev plugins and/or via manifest.toml)

potentially this page's content as I've written it could plausibly live in the DalamudPackager readme? but the additional context about changelogs and icons, if I get round to writing it, definitely doesn't fit there

(sidenote: it looks like existing files are hard-wrapped at 80 columns but I can't see an editorconfig or similar to encourage that; I can add one if desired)

[the DalamudPackager source]:
https://github.com/goatcorp/DalamudPackager/blob/084f66e6af7edbf8a45820590ca71765376b901c/DalamudPackager/DalamudPackager.cs#L303
[the properties Dalamud will load]:
https://github.com/goatcorp/Dalamud/blob/532781308d6291a2c0117e0e73a8252358e2d91a/Dalamud/Plugin/Internal/Types/PluginManifest.cs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should link to IPluginManifest instead. It'll have all of the fields documented, instead of inheritdoc. I would also prefer if you linked to the Dalamud source as the "main" point.

@sersorrel
Copy link
Author

Changed the link to IPluginManifest, and added some details on changelogs.

I'm not 100% sure about the note about PR descriptions not being visible in the plugin installer – would be good to get confirmation that I've correctly interpreted the various related messages on Discord, XLWebServices/Plogon code, and the current changelogs of various plugins. (There's some further detail about nofranz that I considered documenting, but again it's annoyingly complex behaviour like "depending on whether nofranz is in the PR or in another location, and depending on whether it's at the start of the description or not, it will/won't post the Discord message and/or use the PR description as the changelog" that I don't really have any way of testing or verifying, and that most people won't make use of anyway.)


:::warning

If you'd like your changelog to be visible in the plugin installer, you must
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't accurate, changelogs in PR descriptions will also show up in the installer. At the very least they will be in the changelogs tab, if they're not in the installed plugins menu, that would be a bug - should be a simple backend change though.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Turns out the situation is more complex than either of us thought.

Take Daily Duty v4.0.6.1 (goatcorp/DalamudPluginsD17#2930) as an example. There's no changelog in manifest.toml or in the plugin manifest, but there is a PR description, and it does not contain nofranz.

Plugin updates are usually automatically posted in the XIVLauncher & Dalamud
Discord server. If you'd rather write an announcement message yourself, you can
prevent the automatic post by starting your Pull Request description with the
word `nofranz`. Note that any changelog you write in `manifest.toml` or the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's fine to remove this if we fix the behavior I mentioned above, to avoid confusion. The selected changelog at time of check-in should be what's displayed everywhere, and nothing if there is none.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather document the current state of things, rather than confuse the situation more by describing the way it should work in an ideal world.

@sersorrel
Copy link
Author

won't be able to get this updated til next week, sorry; I'll recheck what the state of things is then

@sersorrel
Copy link
Author

i have no idea what the current state of anything is and no motivation to check, unless someone else does or goat remembers what he changed then this will just continue to languish

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