-
Notifications
You must be signed in to change notification settings - Fork 153
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
Add support for Terraform package type #354 #380
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
recheck |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this contribution @nevenr!
Please consider my inline comments.
|
||
@Override | ||
RepositorySettings getRepositorySettings(RepositoryType repositoryType) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (repositoryType == RepositoryTypeImpl.REMOTE) { | ||
def settings = new TerraformRepositorySettingsImpl() | ||
settings.with { | ||
repoLayout = defaultLayout | ||
vcsType = VcsType.GIT | ||
vcsGitProvider = VcsGitProvider.GITHUB | ||
terraformRegistryUrl = "https://registry.terraform.io" | ||
terraformProvidersUrl = "https://releases.hashicorp.com" | ||
remoteRepoLayoutRef = defaultLayout | ||
} | ||
return settings | ||
} | ||
|
||
if (repositoryType == RepositoryTypeImpl.VIRTUAL) { | ||
def settings = new TerraformRepositorySettingsImpl() | ||
settings.with { | ||
repoLayout = moduleLayout | ||
} | ||
return settings | ||
} | ||
|
||
if (repositoryType == RepositoryTypeImpl.FEDERATED) { | ||
def settings = new TerraformRepositorySettingsImpl() | ||
settings.with { | ||
terraformType = TerraformRepositorySettings.TerraformType.module | ||
repoLayout = moduleLayout | ||
} | ||
return settings | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use switch-case here
import org.jfrog.artifactory.client.model.repository.settings.vcs.VcsGitProvider; | ||
import org.jfrog.artifactory.client.model.repository.settings.vcs.VcsType; | ||
|
||
public interface TerraformRepositorySettings extends RepositorySettings{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
public interface TerraformRepositorySettings extends RepositorySettings{ | |
@JsonIgnoreProperties(ignoreUnknown = true) | |
public interface TerraformRepositorySettings extends RepositorySettings { |
|
||
public interface TerraformRepositorySettings extends RepositorySettings{ | ||
|
||
// local and federated settings |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's capitalize all new comments
// local and federated settings | |
// Local and federated settings |
- Changes based on PR jfrog#380 feedback
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @nevenr!
@nevenr |
Please review.
Thanks.
Regards,
N