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

Adapt the column name from "date" to "time" #3

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

ckittl
Copy link
Member

@ckittl ckittl commented Apr 21, 2021

Resolves #1 (sorry for wrong branch naming).

Closing this pull request should only be done, after actual databases have been revised.

@ckittl ckittl added the enhancement New feature or request label Apr 21, 2021
@ckittl ckittl self-assigned this Apr 21, 2021
@ckittl
Copy link
Member Author

ckittl commented Apr 21, 2021

!test

2 similar comments
@ckittl
Copy link
Member Author

ckittl commented Apr 21, 2021

!test

@ckittl
Copy link
Member Author

ckittl commented Apr 21, 2021

!test

@Column(name = "datum", nullable = false)
private ZonedDateTime date;
@Column(name = "time", nullable = false)
private ZonedDateTime time;

@Id
@ManyToOne(cascade = CascadeType.DETACH)
Copy link
Member Author

Choose a reason for hiding this comment

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

Don't know how you feel about this. As the many-to-one relation has no @JoinColumns annotation, it defaults back to coordinate_id, as this field ist named coordinate and the database id of CoordinateModel is id. However, this is quite implicit and I would like it to be more explicit.


def curlByPR(String prId, String orgName, String repoName) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rename prId to prNo as it is called in line 625 and the method extractPrNumber also indicate that it is the number we are actually after.

Comment on lines +625 to +628
String response = curlByPR(prNo, orgName, repoName)
log("i", "API response:" + response)
def jsonResponse = readJSON text: response
def branchName = jsonResponse.head.ref
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this actually do the same thing as the old implementation?

@t-ober
Copy link
Contributor

t-ober commented Mar 24, 2022

Maybe you can have a short look at the Jenkinsfile changes and confirm that this works as I'm not too knowledgable there.

Otherwise I would rename the databases and we can merge the changes.

@sonatype-lift
Copy link

sonatype-lift bot commented Oct 12, 2022

⚠️ 2 God Classes were detected by Lift in this project. Visit the Lift web console for more details.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adapt date column name
2 participants