search

noi-techpark / odh-mentor-otp

4 stars 8 forks source link

fix popup bug in Safari browser #149

Closed stefanocudini closed 1 year ago

stefanocudini commented 1 year ago

small usability fix for Apple users requires only a journey image rebuild

hi @RudiThoeni before merging this PR maybe it is good to check that we are aligned.

I saw that you changed the branch system can you confirm that your old development is now main?

I think it is very strange that all these commits are not included

RudiThoeni commented 1 year ago

Hi @stefanocudini yes we changed branch strategy main is the old development branch prod is the old master branch....

for this commits not sure why they are not included i mentioned it here https://github.com/noi-techpark/odh-mentor-otp/issues/145#issuecomment-1181732595

i think we did not integrate it yet... but lets do it now

RudiThoeni commented 1 year ago

we never integrated this PR https://github.com/noi-techpark/odh-mentor-otp/pull/146 (it is closed now)

stefanocudini commented 1 year ago

we never integrated this PR #146 (it is closed now)

this is no longer necessary it was simply a commit you included in this PR: https://github.com/noi-techpark/odh-mentor-otp/pull/147

stefanocudini commented 1 year ago

Hi @stefanocudini yes we changed branch strategy main is the old development branch prod is the old master branch....

for this commits not sure why they are not included i mentioned it here #145 (comment)

i think we did not integrate it yet... but lets do it now

tomorrow I will do some local tests by rebuilding the images including your latest commits, I don't think there are any problems because you have made only infrastructure changes. My subsequent commits I have updated various libraries and docker images.

let's wait for this tests before doing merge.

the PR I sent now is just a small fix for web interface journey, it's not that important.

RudiThoeni commented 1 year ago

sorry i saw your comment to late and tried to merge........

now the deploy is not working because of the .env.example is now renamed to _dot.env.example if i remember right we had this already one time and now this change is inside again. I can change it and try a deploy but i think i will wait for your feedback tommorrow.........

What i meant before wat that the closed PR 146 already had all of this commits (51) inside and we never integrate it....

stefanocudini commented 1 year ago

now the deploy is not working because of the .env.example is now renamed to _dot.env.example if i remember right we had this already one time and now this change is inside again. I can change it and try a deploy but i think i will wait for your feedback tommorrow.........

now I fix it quickly i send you a pr just for that, yes I remember this too, it is strange that it appears again now they used an absolutely different name for this file did you find any other build problems for the other containers?

What i meant before wat that the closed PR 146 already had all of this commits (51) inside and we never integrate it....

yes that you can ignore 146 I sent it by mistake

RudiThoeni commented 1 year ago

Hi stefano, thx for doing the fix, cannot say that i noticed other things because the deployment stopped at beginning because of this renamend env file.... but since we have the testing environment lets try and we will see....