added the new workflow that automatically puts the labels in PR #741
Conversation
|
Hi @Ronitsabhaya75 thanks for the contribution, this is super helpful! Looks like the labeler workflow failed due to some syntax errors in the configuration file. |
|
sure I can fix those issues thank you so much. |
|
@katiewasnothere can you please check now by run action |
.github/labeler.yml
Outdated
| "area: build": | ||
| - Sources/ContainerBuild/**/* | ||
| - Makefile | ||
| - Package.swift | ||
| - Package.resolved | ||
| - Protobuf.Makefile |
There was a problem hiding this comment.
I think this area is accidentally a combination of container image builder components, project building components, and project dependencies. A large majority of PRs will likely either modify the makefiles or the Package.* files, so I think we can exclude those files from a label category to avoid extra noise.
We should change this to:
area:builder for code related to image building. That would include
- Sources/ContainerBuild//*
- Sources/NativeBuilder//*
.github/labeler.yml
Outdated
| "tooling": | ||
| - scripts/**/* | ||
| - licenserc.toml | ||
| - Makefile | ||
| - Protobuf.Makefile |
There was a problem hiding this comment.
Same as my comment above, I think we can remove this category entirely.
.github/labeler.yml
Outdated
| # Dependencies | ||
| "dependencies": | ||
| - Package.swift | ||
| - Package.resolved |
There was a problem hiding this comment.
Same as my comment above, I think we can remove this category entirely.
.github/labeler.yml
Outdated
| "c/c++": | ||
| - Sources/CVersion/**/* | ||
| - "**/*.c" | ||
| - "**/*.h" |
There was a problem hiding this comment.
Curious what others think here, but I think this category is possibly too small to be useful. We have very little c code and I don't think anyone would really use this label to find or search for PRs.
|
can you give me what category i can put right now instead of all then once it starts for some catgeory then we can add basically rest basically start from small to big |
.github/labeler.yml
Outdated
| # Configuration | ||
| "configuration": | ||
| - config/**/* | ||
| - "**/*.json" | ||
| - "**/*.toml" | ||
| - signing/**/* |
There was a problem hiding this comment.
I'm not sure we need this category
@Ronitsabhaya75 sure thing. I think the easiest category would be the CLI one. So you could remove all of them except that one for now while we get this running. |
sure once I got time after my interview i can fix this thing |
|
@katiewasnothere I have changed to CLI only now we check if this works then we can merge |
|
@katiewasnothere really sorry to disturb you but as per your changes requested I updated the file |
|
@Ronitsabhaya75 The changes look good to me. Do you want to merge this PR as it is now or did you want to add more changes to it? |
|
@katiewasnothere we can merge this pr and is it possible that i can get your email or linkedin for reaching out if there are any intern opportunities avialble within your team |
|
@Ronitsabhaya75 Can you update to sign the commits? When the PR is merged, the commits will be squashed, but we still need to signature on every commit. |
…team to review the pr based on the lables
…le#742) ## Type of Change - [x] Bug fix ## Motivation and Context We setup logging for the services in `container` using OSLog. See [here](https://github.com/apple/container/blob/73709232d2705b7008b7380fe90a373059b6074a/Sources/Helpers/APIServer/APIServer.swift#L31). This makes it unnecessary to redirect stderr and stdio for these services. Additionally, there are some cases where failure to open or write to the path given for StandardErrorPath or StandardOutPath in a service's plist could result in a failure to start a service through `launchctl`. Related to apple#713 ## Testing - [x] Tested locally Signed-off-by: Kathryn Baldauf <[email protected]>
Fixes apple#692 This PR resolves a race condition in `ContainersService.create()` that occurs when creating containers with the same name concurrently.
- Closes apple#466. ## Type of Change - [x] Bug fix - [ ] New feature - [ ] Breaking change - [ ] Documentation update ## Motivation and Context Proxy logic worked well enough for CI but broken in general. ## Testing - [x] Tested locally - [x] Added/updated tests - [ ] Added/updated docs
Closes apple#416. ## Type of Change - [ ] Bug fix - [ ] New feature - [x] Breaking change (possibly, I would think we're just going to leave an unused `.build` directory behind which we can document, but can run more tests next week). - [ ] Documentation update ## Motivation and Context We have nothing to hide? ## Testing - [x] Tested locally - [ ] Added/updated tests - [ ] Added/updated docs
Ronitsabhaya75
force-pushed
the
main
branch
2 times, most recently
from
bfb489c to
1549f9b
Compare
|
@katiewasnothere can you reapprove this merge its not letting me merge |
|
@Ronitsabhaya75 It looks like the containerization version was decremented in Package.swift. Can we remove that? |
Updated the originHash and containerization package version.
|
@katiewasnothere I changed the decremented swift package and resolved the conflicts you can review this |
|
@katiewasnothere I cant see the button for the merging it shows reviewers with writing access can merge |
|
@Ronitsabhaya75 The repository has a number of rules in place for security reasons. One of those is that only maintainers can merge PRs. There is also a rule set that when a new commit is pushed to the PR, the PR will require a new approval from a maintainer and the Github Actions need to be run again (running the Github Actions also requires a separate approval from a maintainer). In general, you probably don't need to update the branch with main unless there have been other changes in other PRs merged to the same code area since your PR was opened or its been a while (like weeks, months, etc) since the PR was opened/looked at last. Since this PR just contains new files, you likely don't need to update with the main branch. I've kicked off the actions and I'm just waiting for them to finish running, then I'll merge the PR. |
…PR" (#776) Reverts #741 The action does not have permissions to add labels. Reverting this while we investigate the right fix @Ronitsabhaya75 heads up
4 participants
Type of Change
@dcantah can you please review this pr and suggest me if any changes require