Skip to content
This repository has been archived by the owner on Jan 14, 2025. It is now read-only.

Merge upstream changes #1

Open
wants to merge 33 commits into
base: mage-os-mirror
Choose a base branch
from

Conversation

johnhughes1984
Copy link
Member

No description provided.

johnhughes1984 and others added 30 commits October 11, 2022 22:30
* added sudo to fix permissions issue

* Fix for missing python in lib in new version

* Update .gitpod.Dockerfile

* Update .gitpod.Dockerfile
* Updated PHP and Magento version .gitpod.Dockerfile

* disabled both 2FA modules
* Updated PHP and Magento version .gitpod.Dockerfile

* disabled both 2FA modules
* Update .gitpod.yml to improve readability and load times

* Reverted init command for installing new Magento instance

* Removed name section so command runs after composer install steps

* removed task name as process didn't run under this

* Cleaned up M2 install script

* Moved create db call

* Added additional 2FA modules
* Added Cypress support

* installed cypress on init command:

* added npm install cypress
* Added required Mailpit ports (#17)

* Added name and description for Nginx Magento application

* Added docker run for starting mailpit

* Added mailpit SMTP config to Magento

* Update .gitpod.yml named ports
Tested and is all functional, I'll add some instructions on the readme file on how to add the Github secrets for the auth.json in the correct format.
Testing upgrading Magento and services
Fixed ES version
Fixed hardcoded ES version value
Replaced dynamic ES version on build
* Update .gitpod.yml

Suppress port open notifications

* Ignored ports by default, fixed mailhog failing on restart

* Update .gitpod.yml

Removed mailpit server port config, only client is useful for users
Reverting ES version increase to get build progressing
Fix percona missing
Updated to latest security patch
Added ENV to Enable Cache Clean by default
Install Mage Cache Cleaner if required, start if available or close terminal
Remove disabling csp as it's a required module now
Remove duplicate disable command
fix if else syntax and missing ;
await nginx starting
updated to use custom gp sync event
Increased admin session lifetime
Upgrade/php8.3 
- Upgrade Magento to 2.4.7-p1
- PHP 8.3
- Mage Cache Cleaner
- Elastic search config fix (removed hard coded values)
- Increase admin session lifetime
- Removed disabling CSP module
@@ -2,18 +2,19 @@ FROM gitpod/workspace-full:latest

# Magento Config
ENV INSTALL_MAGENTO YES
ENV MAGENTO_VERSION 2.4.4
ENV MAGENTO_VERSION 2.4.7-p1

Choose a reason for hiding this comment

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

Suggested change
ENV MAGENTO_VERSION 2.4.7-p1
ENV MAGENTO_VERSION 2.4.7-p3

hey @johnhughes1984, would it be possible to update this to 2.4.7-p3?

@fballiano
Copy link

@johnhughes1984 I don't have the privileges to fix the conflicts, would you be able to take a look at it? I'd try to merge this as quickly as possible afterwards :-)

@fballiano
Copy link

@rhoerr @sprankhub @Vinai could somebody give me privileges on this repo so that I could fix the conflicts and get this merged?

@rhoerr
Copy link

rhoerr commented Dec 23, 2024

I'll look into this later if needed. Probably we need to define a CODEOWNERS for the repo via Terraform.

@rhoerr
Copy link

rhoerr commented Dec 24, 2024

I granted infrastructure write access on this repo until Terraform is worked out. That should help.

@fballiano
Copy link

Unfortunately I still don't have privileges to fix the conflicts, @johnhughes1984 would you fix them and I'll try to merge it afterwards as soon as possible?

@sprankhub
Copy link

In order to fix this existing PR, you'd need permissions on https://github.com/develodesign/magento-gitpod/tree/main, right? I don't think we can fix this on our side unless we cherry-pick the commits and prepare a new PR. But I'm sure we can get the support from the right people if we approach them and wait a bit :)

@fballiano
Copy link

@sprankhub usually if the PR is created with "enable admins to edit this PR", all the admins can push only to the branch of the PR. But I don't think I'm an admin of this repo.

Probably it's easier to just close this one and recreate it from scratch.

@rhoerr
Copy link

rhoerr commented Jan 2, 2025

Given Simon's point about upstream permissions, I don't think it's realistic to us to have a branch that directly tracks upstream along with our customizations. Here, the source (develodesign/magento-gitpod) isn't a fork, it's the upstream repository we're based on.

I think we have three options:

  1. Have a branch that tracks upstream continually, and then run PRs/merges from that to our customized branch. That way we can get the changes from upstream to local without conflicts, and we have permissions to handle the conflicts on merge to our primary branch (I think?).
  2. OR, set this up with the upstream merge process we use to mirror and modify Magento repositories. I'm not sure how complicated this would be, if at all?.
  3. OR, contribute official Mage-OS support directly to the upstream magento-gitpod repository, so no separate repository is needed at all.

Thoughts?

@sprankhub
Copy link

I didn't look into this in detail, but given your input, I wonder why we need two repositories after all? What is included here, which isn't in the develodesign version?

@rhoerr
Copy link

rhoerr commented Jan 3, 2025

I didn't look into this in detail, but given your input, I wonder why we need two repositories after all? What is included here, which isn't in the develodesign version?

Mostly just changes to the readme and the composer repository: develodesign/magento-gitpod@main...mage-os:magento-gitpod:mage-os-mirror

@sprankhub
Copy link

Well, then, IMHO, we should delete our version and, if we want to have it, we convince Develo to fully migrate / move their repository into the Mage-OS namespace.

@fballiano
Copy link

If we don't need to keep a fork (but I'd say a real fork in this case) just as a backup, with no modifications, then I'd also think we should remove this repo. people could collaborate directly with upstream and we'd not be duplicating and thinning resources.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants