Skip to content

Conversation

mad-briller
Copy link
Contributor

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 👼

@mad-briller
Copy link
Contributor Author

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

@williamdes
Copy link
Member

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

Copy link
Member

@williamdes williamdes left a comment

Choose a reason for hiding this comment

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

💯

@codecov
Copy link

codecov bot commented Aug 21, 2021

Codecov Report

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

Impacted file tree graph

@@            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
Copy link
Member

williamdes commented Aug 21, 2021

Hope my Github naivety wasn't too frustrating 👼

Now worries nothing wrong :)

You signed with GPG, I LOVE it!

@mad-briller
Copy link
Contributor Author

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
Copy link
Member

Thank you too!
Will merge this soon

@williamdes williamdes self-assigned this Aug 21, 2021
@williamdes williamdes added this to the v5.4.2 milestone Aug 21, 2021
@williamdes williamdes added the bug Something isn't working label Aug 21, 2021
@williamdes williamdes changed the title FIX: Not url encoding the version string used for navigation. Fix - Not url encoding the version string used for navigation. Aug 22, 2021
@williamdes williamdes changed the title Fix - Not url encoding the version string used for navigation. Fix - Not url encoding the version string used for navigation Aug 22, 2021
@williamdes williamdes merged commit 359171b into code-lts:main Aug 22, 2021
@williamdes
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

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
Copy link
Member

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
Copy link
Contributor Author

mad-briller commented Nov 29, 2021

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

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

@williamdes
Copy link
Member

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

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

Labels

bug Something isn't working

Development

Successfully merging this pull request may close these issues.

2 participants