Skip to content

Turn fetch_if_missing into field of struct repository - #662

Closed
ghost wants to merge 4 commits into
masterfrom
unknown repository
Closed

Turn fetch_if_missing into field of struct repository#662
ghost wants to merge 4 commits into
masterfrom
unknown repository

Conversation

@ghost

@ghost ghost commented Oct 20, 2019

Copy link
Copy Markdown

Hi GIT contributors !
I am Shubham Ghule.I have resolved the problem of extern integer fetch_if_missing by putting it in the structure repository as a field.Made the relevent changes suggested by @dscho.
I'm looking forward to your review.
Special thanks to @dscho.
with regards,
Shubham.

Shubham Ghule added 4 commits September 22, 2019 19:19
Signed-off-by: Shubham Ghule <[email protected]>
turn the fetch_if_missing into field of repositry
made the relevant changes
@dscho

dscho commented Oct 21, 2019

Copy link
Copy Markdown
Member

Welcome, and thanks for working on this!

It seems that there are quite a few merge conflicts, would you mind rebasing this to the current master?

Please make sure that your commit message focuses on answering the question "why?" more than on "how?", and that it wraps at <= 76 columns per line. Also do make sure to add your sign off with a valid email address.

changed fetch_if_missing from global to struct repo

There is a strong preference for using the "imperative mood", please do use it in your commit messages. In general, you should find inspiration how to improve the commit messages here: https://git-scm.com/docs/SubmittingPatches#describe-changes

Finally send the patch to the mailing list for review. You can use GitGitGadget, submitGit or send it manually.

@dscho

dscho commented Oct 30, 2019

Copy link
Copy Markdown
Member

Whoa, now I am confused.

I just realized that you opened a GitGitGadget PR at gitgitgadget#351, then closed it, only to open a PR here.

But the Git project does not accept PRs. Only patch submissions via the Git mailing list. GitGitGadget owes its existence to the mission to help convert PRs to such submissions, so it is not at all clear why the GitGitGadget PR was closed, in favor of a Git PR (which will never be accepted).

What's up with that, @dev-shubham1?

@dscho

dscho commented Nov 12, 2019

Copy link
Copy Markdown
Member

@dev-shubham1 gentle ping?

@dscho

dscho commented Dec 6, 2019

Copy link
Copy Markdown
Member

It actually looks as if the Git project pursues a different direction: getting rid of those assignments altogether (as they are not thread-safe). See e.g. fce9e83

@dscho

dscho commented Dec 8, 2019

Copy link
Copy Markdown
Member

@dev-shubham1 are you interested in this PR at all anymore?

@ghost

ghost commented Dec 11, 2019

Copy link
Copy Markdown
Author

@dev-shubham1 are you interested in this PR at all anymore?
On it.Can you elaborate what I have to do further ?

@dscho

dscho commented Dec 13, 2019

Copy link
Copy Markdown
Member

Can you elaborate what I have to do further ?

Imitate fce9e83, i.e. getting rid of fetch_if_missing assignments altogether (and instead using OBJECT_INFO_SKIP_FETCH_OBJECT)

@dscho

dscho commented Jan 25, 2020

Copy link
Copy Markdown
Member

@dev-shubham1 gentle ping?

@ghost

ghost commented Feb 15, 2020

Copy link
Copy Markdown
Author

@dev-shubham1 gentle ping?

got rid of fetch_if_missing assignments as suggested.What next ?

@dscho

dscho commented Feb 15, 2020

Copy link
Copy Markdown
Member

got rid of fetch_if_missing assignments as suggested.What next ?

Force-push?

@ghost

ghost commented Feb 19, 2020

Copy link
Copy Markdown
Author

Imitate fce9e83, i.e. getting rid of fetch_if_missing assignments altogether (and instead using OBJECT_INFO_SKIP_FETCH_OBJECT)

OBJECT_INFO_SKIP_FETCH_OBJECT is also a global variable but the initial aim was to reduce global variables right ?

@dscho

dscho commented Feb 19, 2020

Copy link
Copy Markdown
Member

OBJECT_INFO_SKIP_FETCH_OBJECT is also a global variable

No, it is a constant. The idea is that it is used in a flag that is passed down through the call path that needs it. See e.g. e362fad#diff-92b149d6fe7eece2bd0ff0fb1ed33a6fR65-R66.

Note: the OBJECT_INFO_* constants are all powers of two so that they can be combined. See e.g. 6462d5e#diff-7891df559f581cdcc9168363e0f932a1R676-R677

@dscho

dscho commented Apr 13, 2020

Copy link
Copy Markdown
Member

Shall we close this in favor of gitgitgadget#606?

@ghost

ghost commented Apr 29, 2020

Copy link
Copy Markdown
Author

Ya Sure

@ghost ghost closed this Apr 29, 2020
derrickstolee pushed a commit to derrickstolee/git that referenced this pull request Jun 28, 2024
Use federated authentication with GitHub Actions and Azure Entra ID for
the Azure login commands during `build-git-installers.yml` builds.

This will allow us to drop the use of a client secret to authenticate as
the signing identity for Trusted Code Signing.

The `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, and `AZURE_SUBSCRIPTION_ID`
secrets have already been added to the `release` environment, and a test
of the `azure/login` step using this mechanism and a subsequent `az`
command has been successfully demonstrated here:
https://github.com/microsoft/git/actions/runs/9652892561/job/26624014573
derrickstolee pushed a commit to derrickstolee/git that referenced this pull request Jul 19, 2024
Use federated authentication with GitHub Actions and Azure Entra ID for
the Azure login commands during `build-git-installers.yml` builds.

This will allow us to drop the use of a client secret to authenticate as
the signing identity for Trusted Code Signing.

The `AZURE_CLIENT_ID`, `AZURE_TENANT_ID`, and `AZURE_SUBSCRIPTION_ID`
secrets have already been added to the `release` environment, and a test
of the `azure/login` step using this mechanism and a subsequent `az`
command has been successfully demonstrated here:
https://github.com/microsoft/git/actions/runs/9652892561/job/26624014573
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants