Skip to content
This repository has been archived by the owner on Sep 12, 2024. It is now read-only.

MARP-59 migrate the marketplace from ivyteam jenkins to GitHub #30

Conversation

tutn-axonivy
Copy link
Contributor

No description provided.

Copy link

github-actions bot commented Mar 22, 2024

Test Results

107 tests   107 ✅  38s ⏱️
 16 suites    0 💤
  1 files      0 ❌

Results for commit f884194.

♻️ This comment has been updated with latest results.

@@ -339,6 +339,7 @@ private function assetBaseUrl_versionized(string $version): string
{
$key = $this->product->getKey();
$artifactId = $this->product->getProductArtifactId();
echo "/market-cache/$key/$artifactId/$version";
Copy link
Contributor

Choose a reason for hiding this comment

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

remove echo

Copy link
Contributor

@ntqdinh-axonivy ntqdinh-axonivy left a comment

Choose a reason for hiding this comment

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

LGTM

@tutn-axonivy tutn-axonivy merged commit 9c335e0 into master Mar 27, 2024
3 checks passed
@tutn-axonivy tutn-axonivy deleted the MARP-59-Migrate-the-Marketplace-from-Ivyteam-Jenkins-to-Github branch March 27, 2024 07:31
@ivy-rew
Copy link
Member

ivy-rew commented Mar 27, 2024

Hi @tutn-axonivy
Cool that you start to build pipelines on github. Note that the jenkins-pipeline is still in charge though ... builds are started on PRs automatically ... so if there is an error and you merge anyway, we have red builds to analyze.
#31

@ivy-rew
Copy link
Member

ivy-rew commented Mar 27, 2024

As you are doing first steps in this repo I think you should let the work review by the initial contributors. Especially @alexsuter in this repo.
This also applies to other market-repos too ... just involve the main contributor on PRs.

@alexsuter
Copy link
Member

I like that you have formatted the code with .editorconfig. But please if you make such reformattings then do this in an own PR. So that we see that this PR is only a formatting PR and should not change the behavior.
You made 108 commits and merged it to master. This makes the history unreadable and unusable. Such PRs should be squashed before your merge.

Do we have any behavior changes in this PR?

@alexsuter
Copy link
Member

You changed the Dockerfile which is currently in charge.

@axonivy-market axonivy-market deleted a comment from azjustin1 Mar 27, 2024
@tutn-axonivy
Copy link
Contributor Author

I did some tricks to make it run in my local. I'm so sorry for not discarding it when committing the code.

@ivy-rew
Copy link
Member

ivy-rew commented Mar 27, 2024

I did some tricks to make it run in my local. I'm so sorry for not discarding it when committing the code.

No problem; we're glad you're contributing after all. 🤝
Let's learn to get better on the way: maybe you could make a retro topic out this story, to establish contribution guidelines within team Octopus?

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.

4 participants