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

Remove ident ID value #400

Merged
merged 1 commit into from
Nov 10, 2023
Merged

Conversation

tjwatson
Copy link
Contributor

No description provided.

@tjwatson tjwatson requested a review from HannesWell November 10, 2023 14:14
@iloveeclipse
Copy link
Member

Link to the issue: #399

Copy link

Test Results

     24 files  ±0       24 suites  ±0   11m 20s ⏱️ +24s
2 150 tests ±0  2 105 ✔️ ±0  45 💤 ±0  0 ±0 
2 194 runs  ±0  2 149 ✔️ ±0  45 💤 ±0  0 ±0 

Results for commit 3c7ac92. ± Comparison against base commit 1201365.

@vogella
Copy link
Contributor

vogella commented Nov 10, 2023

PMC approved (in case you need that)

@tjwatson tjwatson merged commit 814024b into eclipse-equinox:master Nov 10, 2023
19 of 23 checks passed
@HannesWell
Copy link
Member

Thanks Tom.

Indeed I used EGit and had no issue with that.
I just copied the original sources from Maven-Central.

@iloveeclipse
Copy link
Member

Unfortunately the SDK build failed, see eclipse-platform/eclipse.platform.releng.aggregator#1534

@iloveeclipse
Copy link
Member

@tjwatson: Is there any reason to keep .gitattributes that cause such confusion?
Should we remove the ident sections from https://github.com/eclipse-equinox/equinox/blob/master/.gitattributes?
See git manual: https://git-scm.com/docs/gitattributes#:~:text=ident,Id%24%20upon%20check-in.

@tjwatson tjwatson deleted the removeIdentValue branch November 13, 2023 13:53
@tjwatson
Copy link
Contributor Author

@tjwatson: Is there any reason to keep .gitattributes that cause such confusion?

We can consider removing the ident value from the .gitattributes. It only is an issue for the osgi source we include in our repo since we don't use $Id$ in org.eclipse source. But it will mean we either checkin source that has the Id filled in or it will be blank.

@iloveeclipse
Copy link
Member

But it will mean we either checkin source that has the Id filled in or it will be blank.

What is the problem with blank id? It is comment anyway, and I doubt the commit hash is that important for the sources shipped with a specific bundle version.

@tjwatson
Copy link
Contributor Author

What is the problem with blank id? It is comment anyway, and I doubt the commit hash is that important for the sources shipped with a specific bundle version.

I'm not suggesting it is a problem, only that we have to decide. In the past you could diff the equinox source with the osgi source repo and they would show no differences because the Id would get filled in correctly on checkout. If we use a blank Id then the diff that I usually run to make sure I got the right source will show a difference. In the end it is unfortunate that we have to work around a many year old bug in jGit. This long running bug has simply got me use to never using eGit to commit anything, ever.

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.

4 participants