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

Provide default to get_fields #19

Merged
merged 5 commits into from
Nov 13, 2024
Merged

Provide default to get_fields #19

merged 5 commits into from
Nov 13, 2024

Conversation

llrs-roche
Copy link
Contributor

This patch was proposed by Pawel to fix issues like https://github.com/insightsengineering/teal/actions/runs/11794622576/job/32853793903?pr=1406 and https://github.com/insightsengineering/teal/actions/runs/11794595252/job/32852528908?pr=1357 where the message is:

Modify DESCRIPTION file (development)
  ℹ Loading metadata database
  ✔ Loading metadata database ... done
  
  ! Failed to resolve dependencies.
  Error:
  * local::teal/:
    * Can't install dependency teal.data (>= 0.6.0.9017) (>= 0.5.1.9009) (>= 0.5.0.9015) (>= 0.3.1.9004)
    * Can't install dependency teal.slice (>= 0.5.1.9009) (>= 0.5.0.9015) (>= 0.3.1.9004)
    * Can't install dependency teal.code (>= 0.5.0.9015) (>= 0.3.1.9004)
    * Can't install dependency teal.reporter (>= 0.3.1.9004)
  End of error
  Failed refs: teal.data, teal.slice, teal.code, and teal.reporter.
  Error in val %||% default : Field 'Config/Needs/DepsBranch' not found
  Calls: <Anonymous> -> idesc_get_field -> %||%
  Execution halted

Simple reproducible code:

desc2 <- desc::description$new(file = system.file("DESCRIPTION",
                                              package = "desc"))
desc2$get_field("whatever")
## Error in val %||% default : Field 'whatever' not found

I also added a default on line 165 not originally proposed but I think could avoid this problem in other circumstances.

Not tested (yet) as I'm editing directly from github.

Signed-off-by: Lluís Revilla <[email protected]>
Copy link

github-actions bot commented Nov 12, 2024

✅ All contributors have signed the CLA
Posted by the CLA Assistant Lite bot.

@llrs-roche
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

@llrs-roche
Copy link
Contributor Author

There is no script/bootstrap as indicated on the guidelines https://github.com/insightsengineering/.github/blob/b0a8ef47139d75c047a5b244911b5bf00491f7f8/CONTRIBUTING.md#submitting-a-pull-request

If there is an easy way to test let me know.

@llrs-roche llrs-roche marked this pull request as ready for review November 12, 2024 11:31
action.yaml Outdated Show resolved Hide resolved
action.yaml Outdated Show resolved Hide resolved
action.yaml Outdated Show resolved Hide resolved
action.yaml Outdated Show resolved Hide resolved
@llrs-roche
Copy link
Contributor Author

Hi @pawelru is there anything else to do to fix this? This is blocking the two linked PR.

@llrs-roche
Copy link
Contributor Author

This PR was tested on teal, on a branch that was previously failing before: after the checks work and the workflow continues.

It reports an error solving the dependency but later it solves the dependencies, as expected, and continues with all the steps:

Modify DESCRIPTION file (development)
  ℹ Loading metadata database
  ✔ Loading metadata database ... done
  ! Failed to resolve dependencies.
  Error:
  * local::teal/:
    * Can't install dependency teal.data (>= 0.6.0.9017) (>= 0.5.1.9009) (>= 0.5.0.9015) (>= 0.3.1.9004)
    * Can't install dependency teal.slice (>= 0.5.1.9009) (>= 0.5.0.9015) (>= 0.3.1.9004)
    * Can't install dependency teal.code (>= 0.5.0.9015) (>= 0.3.1.9004)
    * Can't install dependency teal.reporter (>= 0.3.1.9004)
  End of error
  Failed refs: teal.data, teal.slice, teal.code, and teal.reporter.
  ℹ Adding package teal.data (ref: "insightsengineering/teal.data") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.slice (ref: "insightsengineering/teal.slice") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.code (ref: "insightsengineering/teal.code") to the Config/Needs/DepsDev field.
  ℹ Adding package teal.reporter (ref: "insightsengineering/teal.reporter") to the Config/Needs/DepsDev field.
  ℹ Trying to resolve dependencies again.
  ---
  ✔ Dependencies resolved successfully.
Print added field

@cicdguy, can you please make a release for this once merged?

Copy link
Contributor

@pawelru pawelru left a comment

Choose a reason for hiding this comment

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

Looks good 👍 Thank you!

@cicdguy cicdguy merged commit 2e786f2 into main Nov 13, 2024
5 checks passed
@cicdguy cicdguy deleted the dependency_resolution@main branch November 13, 2024 17:54
@github-actions github-actions bot locked and limited conversation to collaborators Nov 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants