code-lts / doctum

A php API documentation generator, fork of Sami

https://doctum.long-term.support/

MIT License

300 stars 32 forks source link

Fix - Not url encoding the version string used for navigation #32

Closed mad-briller closed 3 years ago

mad-briller commented 3 years ago

Me again!

I've resolved the issues outlined in the prior PR #31;

signed the commits using the correct email
no longer made the unnecessary changes to the tests
used the url_encode twig filter rather than creating a property on the Version class

Thanks for your input on the changes, and again for the great utility.

Hope my Github naivety wasn't too frustrating :angel:

mad-briller commented 3 years ago

now Github is complaining my email is undeliverable :sob: i cannot win

williamdes commented 3 years ago

now Github is complaining my email is undeliverable 😭 i cannot win

Do you mean In the emails emails list? If yes I can heave a look as I have some expertise with emails and configs

codecov[bot] commented 3 years ago

Codecov Report

Merging #32 (3990a99) into main (62f3651) will not change coverage. The diff coverage is n/a.

@@            Coverage Diff            @@
##               main      #32   +/-   ##
=========================================
  Coverage     56.34%   56.34%           
  Complexity     1159     1159           
=========================================
  Files            51       51           
  Lines          3012     3012           
=========================================
  Hits           1697     1697           
  Misses         1315     1315

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 62f3651...3990a99. Read the comment docs.

williamdes commented 3 years ago

Hope my Github naivety wasn't too frustrating 👼

Now worries nothing wrong :)

You signed with GPG, I LOVE it!

mad-briller commented 3 years ago

now Github is complaining my email is undeliverable sob i cannot win

Do you mean In the emails emails list? If yes I can heave a look as I have some expertise with emails and configs

I've fixed it so it doesn't show up in the pr anymore, but the email address i originally used was bouncing github emails for some reason, so i changed to a different one and resigned the commit

The GPG signing is a nice touch that i hadn't seen before, will have to investigate with other providers i use.

Thanks for your time on this issue mate

williamdes commented 3 years ago

Thank you too! Will merge this soon

williamdes commented 3 years ago

Do you need this to be released soon ? There is not much to make a version released but it you need one let me know

I updated the 5.4.2-dev phar

mad-briller commented 3 years ago

I can use the 5.4.2-dev phar in our ci setup for now, i'll keep an eye out for when a release is made, thanks!

williamdes commented 3 years ago

I can use the 5.4.2-dev phar in our ci setup for now, i'll keep an eye out for when a release is made, thanks!

By the way if you use GitHub actions I published an action for Doctum

Will try to keep in mind to post a message here

Thank you for contributing!

You can use the subscribe custom to releases only on GitHub

mad-briller commented 3 years ago

unfortunately we use bitbucket + pipelines so i've written something custom, but thanks for the heads up

and good idea on subscribing to releases!

williamdes commented 3 years ago

Hi @mad-briller

I am pushing more changes into 5.x before making a release, could you please try the latest dev phar ?

See: https://github.com/code-lts/doctum#installation

curl -O https://doctum.long-term.support/releases/5-dev/doctum.phar && chmod +x doctum.phar

mad-briller commented 2 years ago

hey @williamdes sorry for the delay, only just seen this,

just tested the new prerelease and it works great, thank you!

williamdes commented 2 years ago

Hi @mad-briller
The version 5.5.0 is out 🎉 This fix is included in the version released