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.

Sign up for GitHub

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

Jump to bottom

Apply autoupdate to silence autogen warnings #5555

Merged
merged 2 commits into from Jul 11, 2023
Merged

Apply autoupdate to silence autogen warnings #5555

merged 2 commits into from Jul 11, 2023

Conversation

MisterDA
Copy link
Contributor

No description provided.

@MisterDA MisterDA force-pushed the autoupdate branch 4 times, most recently from 2f2fac6 to b55e1f9 Compare May 23, 2023 13:16
@MisterDA MisterDA changed the title Run autoupdate to silence autogen warnings Apply autoupdate to silence autogen warnings May 23, 2023
rjbou
rjbou reviewed May 30, 2023
View reviewed changes
Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

I've added an autopudate check in hygiene job.

What is the origin/point of the second commit OPAMCLIVersion.of_string ?

src/client/opamCLIVersion.ml Outdated Show resolved Hide resolved
src/client/opamCLIVersion.ml Outdated Show resolved Hide resolved
@MisterDA
Copy link
Contributor Author

I've added an autopudate check in hygiene job.

Is this why you reverted the first commit? To check that autogen would fail in the CI?

What is the origin/point of the second commit OPAMCLIVersion.of_string ?

Well, I had to debug why opam was failing to parse/extract its version from the configure file. It was nice to print the actual string to see what happened, a bit like "No such file or directory" is enhanced if you actually know which file or directory has not been found.

@rjbou
Copy link
Collaborator

rjbou commented May 30, 2023

I've added an autopudate check in hygiene job.

Is this why you reverted the first commit? To check that autogen would fail in the CI?

Oups, it was to test locally. I'll run some tests on my repo to confirm that the error is triggered.

What is the origin/point of the second commit OPAMCLIVersion.of_string ?

Well, I had to debug why opam was failing to parse/extract its version from the configure file. It was nice to print the actual string to see what happened, a bit like "No such file or directory" is enhanced if you actually know which file or directory has not been found.

Oh i see, for that, it's better to open another PR, to keep them single purpose.

@MisterDA MisterDA mentioned this pull request Jun 1, 2023
Merged
@MisterDA
Copy link
Contributor Author

MisterDA commented Jun 1, 2023

Oh i see, for that, it's better to open another PR, to keep them single purpose.

Done!

@MisterDA
Copy link
Contributor Author

Rebased, I think this is good to go.

@rjbou rjbou added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Jun 19, 2023
@rjbou rjbou removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Jul 11, 2023
@MisterDA
Copy link
Contributor Author

Rebased the PR on the current master.

rjbou
rjbou approved these changes Jul 11, 2023
View reviewed changes
Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

Github action tested & validated, good to go. Thanks!

@rjbou rjbou added this to PR in progress in Opam 2.2.0 via automation Jul 11, 2023
@rjbou rjbou added this to the 2.2.0~alpha2 milestone Jul 11, 2023
@rjbou rjbou moved this from PR in progress to PR finalised (merge with CI) in Opam 2.2.0 Jul 11, 2023
@rjbou rjbou merged commit 1064691 into ocaml:master Jul 11, 2023
27 checks passed
Opam 2.2.0 automation moved this from PR finalised (merge with CI) to Done Jul 11, 2023
@MisterDA MisterDA deleted the autoupdate branch July 11, 2023 14:06
@rjbou rjbou mentioned this pull request Jul 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rjbou rjbou rjbou approved these changes

Assignees
No one assigned
Labels
STATE: READY TO MERGE
Projects
Opam 2.2.0
  
Done
Milestone
2.2.0~alpha2
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MisterDA @rjbou