Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Monorepo] - Proclaim monorepo #4014

Merged
merged 19 commits into from
Sep 25, 2023

Conversation

eapolinario
Copy link
Contributor

@eapolinario eapolinario commented Sep 8, 2023

Describe your changes

This PR proclaims the 7 modules introduced by #4012 as the new place for contribution.

The list of changes:

  1. Rename the modules
  2. Build single-binary and sandbox (both demo and old sandbox) using the local modules
  3. Component CI checks moved from each repo
  4. Changes to releases

Let's talk about each change separately.

Rename the modules

The rules that govern the publishing of multi-module repositories are described in Go Modules Reference - The Go Programming Language, more specifically:

If a module is defined in a subdirectory within the repository, that is, the module subdirectory portion of the module path is not empty, then each tag name must be prefixed with the module subdirectory, followed by a slash. For example, the module http://golang.org/x/tools/gopls is defined in the gopls subdirectory of the repository with root path http://golang.org/x/tools . The version v0.4.0 of that module must have the tag named gopls/v0.4.0 in that repository.

This essentially means that in a subsequent PR, the one where we deal with release, we'll need to generate a few more tags, one per-component.

This PR simply fixes the names of the modules and ensures that go replace directives point to each module. I manually tested the compilation of each component. A subsequent PR is going to fix the single-binary and the components Dockerfiles.

Build single-binary using local modules

Done in this stacked PR.

Component CI checks

Done as part of this stacked PR.

Changes to releases

edit: this is going to come in a separate PR. For now merges to master will not trigger image builds nor releases.

We're going to tag merges to master using semver versionsing, so these new tags will update the patch version and use that to create a new release. Exactly how we do currently in each component. These automatic releases are not going to create new helm releases.

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
pingsutw
pingsutw previously approved these changes Sep 8, 2023
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
…nd-go-replace

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
pingsutw
pingsutw previously approved these changes Sep 13, 2023
* Add dockerfiles

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Build component using reusable workflow

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Add "Components checks" workflow

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix typo

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Rename gh workflow

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Use the correct dockerfile

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Enable endtoend tests

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Use correct path to end2end reusable workflow

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Use unique prefixes for the cache

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Use tmp/tmp

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Be more explicit about the path components docker images are saved to

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Test only overriding datacatalog image

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Comment out actual helm upgrades (i.e. simply run tests)

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Comment out end2end and bring integration tests

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix typo in definition of priorities

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Hardcode go version to 1.19

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Use correct working directory in integration.yml

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Enable flytepropeller integration tests

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Unpack envvars prior to calling reusable workflow

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Enable go_generate.yml

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Enable push_docker_image

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix flytecopilot go generate

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix image tags

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Enable lint and unit tests

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Pass component to lint and unit-tests jobs

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix flytestdlib unit test

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix flyteplugins test

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Build flytescheduler image

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Monorepo  ci checks fix lint (#4032)

* Fix flyteplugins lint errors

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix datacatalog lint errors

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix flyteadmin lint errors

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix flyteaidl lint errors

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix flytepropeller lint errors

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Comment flytecopilot lint and flyteidl unit tests

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

---------

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

---------

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
* Fix single-binary

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix flyteplugins references in single-binary

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Point to local flyteidl in single-binary

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

* Fix flytecopilot references

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>

---------

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Co-authored-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

All modified lines are covered by tests ✅

❗ No coverage uploaded for pull request base (master@6296beb). Click here to learn what that means.

❗ Current head 0acb9d8 differs from pull request most recent head 2f3bd55. Consider uploading reports for the commit 2f3bd55 to get more accurate results

Additional details and impacted files
@@            Coverage Diff            @@
##             master    #4014   +/-   ##
=========================================
  Coverage          ?   59.19%           
=========================================
  Files             ?      549           
  Lines             ?    39512           
  Branches          ?        0           
=========================================
  Hits              ?    23388           
  Misses            ?    13822           
  Partials          ?     2302           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
@eapolinario eapolinario force-pushed the monorepo--rename-modules-and-go-replace branch 2 times, most recently from bb101c0 to 4fdf255 Compare September 13, 2023 22:33
@eapolinario eapolinario changed the title [Monorepo] - Rename modules and add go replace directives [Monorepo] - Proclaim monorepo Sep 14, 2023
Copy link
Member

@pingsutw pingsutw left a comment

Choose a reason for hiding this comment

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

LGTM, but there is a failing test

Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
Signed-off-by: Eduardo Apolinario <eapolinario@users.noreply.github.com>
@eapolinario eapolinario merged commit 73d98d1 into master Sep 25, 2023
40 of 42 checks passed
@eapolinario eapolinario deleted the monorepo--rename-modules-and-go-replace branch September 25, 2023 21:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants