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

feat(model): adds PRE in the FabricType enum #4226

Merged
merged 2 commits into from
Mar 2, 2022

Conversation

sgomezvillamor
Copy link
Contributor

@sgomezvillamor sgomezvillamor commented Feb 23, 2022

Our organization uses DEV, PRE, and PRO as standard names for the usual environments.

The missing PRE in the FabricType forces us to do mappings (eg EI --> PRE) everywhere. So kindly requesting adding PRE.

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable)

@github-actions
Copy link

github-actions bot commented Feb 23, 2022

Unit Test Results (build & test)

  70 files  +  1    70 suites  +1   10m 17s ⏱️ + 3m 36s
611 tests +71  552 ✔️ +64  59 💤 +7  0 ±0 

Results for commit a9fc5fe. ± Comparison against base commit 49a8ece.

♻️ This comment has been updated with latest results.

@@ -54,11 +54,11 @@ This is a [known issue](https://github.com/linkedin/rest.li/issues/287) when bui

As we generate quite a few files from the models, it is possible that old generated files may conflict with new model changes. When this happens, a simple `./gradlew clean` should reosolve the issue.

### `Execution failed for task ':gms:impl:checkRestModel'`
### `Execution failed for task ':metadata-service:restli-servlet-impl:checkRestModel'`
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thank you!!!

Copy link
Collaborator

@jjoyce0510 jjoyce0510 left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Thanks!

@shirshanka shirshanka merged commit a2a939d into datahub-project:master Mar 2, 2022
@sgomezvillamor sgomezvillamor deleted the patch-1 branch March 2, 2022 08:29
maggiehays pushed a commit to maggiehays/datahub that referenced this pull request Aug 1, 2022
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.

3 participants