search

StackStorm / stackstorm.com

StackStorm website and blog based on Github static pages.
https://stackstorm.com/
Apache License 2.0
0 stars 4 forks source link

Add Orchestral Case Study page to StackStorm website and add a Partner page URL #11

Closed dalesmith closed 2 years ago

dalesmith commented 2 years ago

This PR is to merge changes that add an Orchestral Case Study page to the StackStorm website as well as place a link to that Case Study page on the StackStorm Partner page.

dalesmith commented 2 years ago

All image files have been placed into an "images" sub-directory to the index.html directory. Thus, all image files are now within this PR with no externally referenced images and all URLs have been adjusted accordingly. @armab Thanks for the very helpful guidance!...much appreciated. :-)

dalesmith commented 2 years ago

Hi Eugen,

There must be a mistake somewhere because the file index.html file that you should have reviewed absolutely, positively was cleaned of all images referring to orchestral.ai as source.

It seems that you are reviewing something very different from what I have intended to for your review.

Anyway, perhaps, there is some error in the way that I did the commit...I will have another look to see if I can figure out what's happening.

Thanks and apologies for this.

Cheers,

Dale

On 8/9/22 19:48, Eugen C. wrote:

@.**** requested changes on this pull request.

Left a comment about the images still referring to orchestral.ai as a source.

You might also need to add a new Orchestral block here: https://stackstorm.com/case-studies/

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r941887438:

Hey @dalesmith https://github.com/dalesmith, looks like there are still lots of images referring to orchestral.ai.

We need absolute paths to the st2 hosted images.

⬇️ Suggested change

Check all the links on this page. Placing them under the |/case study/| works too, but keeping them under |/wp/wp-content/uploads/| where all the other images live would be a bit more consistent.

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r941887580:

Remove

— Reply to this email directly, view it on GitHub https://github.com/StackStorm/stackstorm.com/pull/11#pullrequestreview-1067496705, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2IIXPWUMQIUDA76XRSXFPDVYLU3NANCNFSM553JBFSA. You are receiving this because you were mentioned.Message ID: @.***>

dalesmith commented 2 years ago

Hey Eugen,

Based on your note below, I'm convinced that I must have made an error in my Commit because the links you've listed below absolutely do not exist in my index.html (see attached).

Anyway, all my image file references were changed to the format shown in the attached but clearly you are seeing something else so I need to figure out why.

Dale

On 8/10/22 18:10, Eugen C. wrote:

@.**** requested changes on this pull request.

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r942943734:

this should be hosted on stackstorm.com

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r942943854:

external image

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r942943990:

External image

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r942944182:

External image

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r942944416:

external image

— Reply to this email directly, view it on GitHub https://github.com/StackStorm/stackstorm.com/pull/11#pullrequestreview-1068984446, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2IIXPSQXYQZUS3GCB7LF3DVYQSEDANCNFSM553JBFSA. You are receiving this because you were mentioned.Message ID: @.***>

dalesmith commented 2 years ago

Definitely!...agreed!...was planning to remove all this on the next Commit.

Thanks,

Dale

On 8/10/22 18:13, Eugen C. wrote:

@.**** commented on this pull request.

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r942947988:

These blocks and more below are coming from the WordPress plugins and are best to be removed.

— Reply to this email directly, view it on GitHub https://github.com/StackStorm/stackstorm.com/pull/11#pullrequestreview-1068990218, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2IIXPRU4IEORA7BXCMUWITVYQSP5ANCNFSM553JBFSA. You are receiving this because you were mentioned.Message ID: @.***>

dalesmith commented 2 years ago

Ok Eugen, no problem and great suggestion!

Thanks,

Dale

On 8/11/22 19:05, Eugen C. wrote:

@.**** requested changes on this pull request.

In case-study-orchestral/index.html https://github.com/StackStorm/stackstorm.com/pull/11#discussion_r944004156:

For all the URLs or links that include stackstorm.com, it's better to use the absolute dir/path like this:

⬇️ Suggested change

This will ensure the structure works when someone tries to develop it locally.

— Reply to this email directly, view it on GitHub https://github.com/StackStorm/stackstorm.com/pull/11#pullrequestreview-1070453058, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2IIXPRJNB7225YWPS2ZZETVYWBMVANCNFSM553JBFSA. You are receiving this because you were mentioned.Message ID: @.***>

dalesmith commented 2 years ago

Hi Eugen, Thanks for your patience and guidance here. I hope that I have fixed everything per your recommendations. So, fingers crossed...this time the PR can go through but don't hesitate to let me know if there are additional fixes required.

dalesmith commented 2 years ago

Hi Eugen,

Cool!...thanks for the great feedback and guidance.

I completely missed the Orchestral block on the Case Studies homepage.

I'll have that added sometime today...thanks for the heads-up on that one, very important for discoverability just as you say.

Cheers,

Dale

On 8/16/22 06:50, Eugen C. wrote:

@.**** commented on this pull request.

Hey @dalesmith https://github.com/dalesmith, the page HTML looks good, - great work on polishing it!

The only missing part is an Orchestral block on the https://stackstorm.com/case-studies/ page along with other cases, which will make your case study more discoverable.

— Reply to this email directly, view it on GitHub https://github.com/StackStorm/stackstorm.com/pull/11#pullrequestreview-1073894896, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2IIXPSTMWGRJLWKPXBUT4DVZNW6FANCNFSM553JBFSA. You are receiving this because you were mentioned.Message ID: @.***>

arm4b commented 2 years ago

FYI I'm also adding README instructions for the local development with docker-compose and nginx here: https://github.com/StackStorm/stackstorm.com/pull/12, if that helps.

dalesmith commented 2 years ago

Hi Eugen,

Ok great!..thanks for the helpful tips...much appreciated.

I'll fix the CSS problem now.

Cheers,

Dale

On 8/17/22 18:17, Eugen C. wrote:

@.**** requested changes on this pull request.

Hey @dalesmith https://github.com/dalesmith, the new content block looks good 👍 image https://user-images.githubusercontent.com/1533818/185252904-456fcb79-0447-4a56-b2ca-c678dd8d1a9d.png

However, as a final check I just found that after you removed some CSS references the case study looks this way:

image https://user-images.githubusercontent.com/1533818/185251728-de898194-cfca-420f-bcf7-bf25170f881e.png

Looks like the page was referring to some CSS styles that existed on orchestral.ai, but not on stackstorm.com. The previous version (before f0bb5db https://github.com/StackStorm/stackstorm.com/commit/f0bb5dbeab6324c84c42fa1f356a1b4c46110792) looked OK.

Here are the 3 CSS files that are expected by your HTML:

|<link rel="stylesheet" href="https://orchestral.ai/wp-content/plugins/oxygen/component-framework/oxygen.css?ver=4.0.2" type="text/css" media="all"/> <link rel="stylesheet" href="//orchestral.ai/wp-content/uploads/oxygen/css/2332.css?cache=1658928015&ver=6.0.1" type="text/css" media="all"/> <link rel="stylesheet" href="//orchestral.ai/wp-content/uploads/oxygen/css/universal.css?cache=1658936364&ver=6.0.1" type="text/css" media="all"/> |

Try to refactor the HTML so it doesn't rely on external CSS references but uses existing stackstorm styles.

Probably copy-pasting the HTML body structure from the other case studies and replacing only the text + images would be the easiest path.

— Reply to this email directly, view it on GitHub https://github.com/StackStorm/stackstorm.com/pull/11#pullrequestreview-1076487529, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2IIXPV4ZQ53OQKTL5DIU5TVZVQGPANCNFSM553JBFSA. You are receiving this because you were mentioned.Message ID: @.***>

dalesmith commented 2 years ago

I tested these changes with localhost so hopefully (fingers crossed) this PR will not be troublesome any longer. Thanks for the great guidance and assistance on this @Eugen, much appreciated!

dalesmith commented 2 years ago

I think these last set of changes will do it. As always, don't hesitate to let me know if further work is required.

dalesmith commented 2 years ago

Hi Eugen,

Very strange...I could have sworn that I removed that 'assets' sub-directory.

I must have been very sleepy/tired to have overlooked that.

Anyway, I'll fix that right now...apologies for overlooking that and forcing you through another round of review.

Should be fixed shortly...

Cheers,

Dale

On 8/22/22 19:25, Eugen C. wrote:

@.**** commented on this pull request.

Hi @dalesmith https://github.com/dalesmith, thanks for the changes, the main case study HTML page looks good now

Could you also please delete |assets/2332.css|, |assets/universal.css|, and |assets/universal.css| files? They're not used anymore but are still included in the git.

Outside of that minor file cleanup, everything looks good to me 👍

— Reply to this email directly, view it on GitHub https://github.com/StackStorm/stackstorm.com/pull/11#pullrequestreview-1081285393, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2IIXPQ2PDUJYTA5HBGJFS3V2QD5ZANCNFSM553JBFSA. You are receiving this because you were mentioned.Message ID: @.***>

dalesmith commented 2 years ago

Deleted the 'assets' directory containing (3) css files that are not needed/used.

dalesmith commented 2 years ago

Hi Eugen,

Ok no problem...I understand.

I'll take another stab at it...

Thanks and cheers,

Dale

On 8/18/22 18:08, Eugen C. wrote:

@.**** commented on this pull request.

Hi @dalesmith https://github.com/dalesmith, sorry we can't add 3 new CSS files from the other website because the case study relies on it, otherwise, the stackstorm.com would become inconsistent very quickly.

Instead, the direction is to remove all the unneeded CSS styles from the WordPress leftower in the future (see Cleanup and Merge CSS #8 https://github.com/StackStorm/stackstorm.com/issues/8 issue for more context).

Please try to refactor the HTML so it uses the existing stackstorm styles, like the other case studies. As suggested, copy-pasting the HTML body structure from the other case studies and replacing only the text + images would be the easiest path.

I hope that makes sense in the context of the stackstorm.com website consistency we maintain collectively now.

— Reply to this email directly, view it on GitHub https://github.com/StackStorm/stackstorm.com/pull/11#pullrequestreview-1078064901, or unsubscribe https://github.com/notifications/unsubscribe-auth/A2IIXPSMQNSN6VF7L4QMVMDVZ2X6PANCNFSM553JBFSA. You are receiving this because you were mentioned.Message ID: @.***>

arm4b commented 2 years ago

Hey @dalesmith, the comment you're mentioning was resolved a while ago. The PR was merged almost 2 months ago, no other changes are needed. So no worries!