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

RFC 117: Remove dated tags for epoch branches #117

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

foolip
Copy link
Member

@foolip foolip commented Jul 6, 2022

No description provided.

@clopez
Copy link

clopez commented Jul 6, 2022

I still rely on this tags when there is a issue with WebKitGTK on the CI that needs to be investigated.
However, that issue is 100% of the times some weeks old as much.
So an alternative to this would be to automatically remove tags older than X (being X a few months)

That would be my preferred option (purge automatically old tags), but I'm a sporadic user of this feature, so I'm fine relying instead on the command ./wpt rev-list if you feel is better to get rid completely of this tags.

Copy link
Contributor

@jgraham jgraham left a comment

Choose a reason for hiding this comment

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

I don't believe we rely on this for anything, so no objections on that front, but the RFC doesn't seem to set ouf the case for the change, only that there are alternative ways to get the same information.

@gsnedders
Copy link
Member

The other potential option here would be to use a different namespace for the epoch refs, such as pushing them into refs/epochs/*.

This has the upside that omits them from the default refspec, which means normally users won't constantly see the branches/tags used for them. (We could also do likewise with the merge tags.)

@foolip
Copy link
Member Author

foolip commented Jul 8, 2022

I still rely on this tags when there is a issue with WebKitGTK on the CI that needs to be investigated.

Can you describe how you use the existing tags? Is it to figure out which runs have been triggered in order to find the CI runs in the first place, or is it when you're already looking at a failed CI run and want to find some previous run? Or something else?

@foolip
Copy link
Member Author

foolip commented Jul 8, 2022

I don't believe we rely on this for anything, so no objections on that front, but the RFC doesn't seem to set out the case for the change, only that there are alternative ways to get the same information.

My motivation is to make git fetch and git log less noisy. We've already accumulated 6500 of these tags, and they lump up on certain commits in git log and make the output less regular. So line noise, really.

@jgraham
Copy link
Contributor

jgraham commented Jul 8, 2022

We could also do likewise with the merge tags.

That would be a ~breaking change for the gecko sync (we use the tags in the first instance to associate a commit on master with a PR). Not impossible to update ofc, but not something we should just blindly change.

@jgraham
Copy link
Contributor

jgraham commented Jul 8, 2022

My motivation is to make git fetch and git log less noisy.

Can you update the RFC with this reasoning?

@foolip
Copy link
Member Author

foolip commented Jul 8, 2022

We could also do likewise with the merge tags.

That would be a ~breaking change for the gecko sync (we use the tags in the first instance to associate a commit on master with a PR). Not impossible to update ofc, but not something we should just blindly change.

We also need regular tags to create releases, and release artifacts is how we store the manifests.

I would like to get rid of those tags (and lots of branches) too but that's not for this RFC.

@foolip
Copy link
Member Author

foolip commented Jul 8, 2022

My motivation is to make git fetch and git log less noisy.

Can you update the RFC with this reasoning?

Will do after I've heard from @clopez. It's still possible I've misunderstood what these tags are for.

@gsnedders
Copy link
Member

We could also do likewise with the merge tags.

That would be a ~breaking change for the gecko sync (we use the tags in the first instance to associate a commit on master with a PR). Not impossible to update ofc, but not something we should just blindly change.

We also need regular tags to create releases, and release artifacts is how we store the manifests.

I would like to get rid of those tags (and lots of branches) too but that's not for this RFC.

ah, that's true; yeah, probably worthwhile as a separate discussion

@clopez
Copy link

clopez commented Jul 12, 2022

Can you describe how you use the existing tags? Is it to figure out which runs have been triggered in order to find the CI runs in the first place, or is it when you're already looking at a failed CI run and want to find some previous run? Or something else?

I use them to find the commit that triggered the runs on a given day, so I can check the CI.

  1. For example, I have this report of WebKitGTK runs missed lately
  2. If I want to check what happened on the run of a given day, I go to the wpt repository, and find the tag corresponding to that day. For example, the last run for WebKitGTK would be the one from 11th of Jul (the runs for it do that on a daily basis frequency and right now is the 12th 13:00 UTC so the one for 12th was still not scheduled)
  3. So I find the corresponding tag on the repository: epochs/daily/2022-07-11_02H And I open that commit on the github UI: web-platform-tests/wpt@d3422cc
  4. Then I click on the red cross indicating that some of the tests on the CI failed and check the ones related to webkitgtk on TaskCluster

So I use this tags to map a run on a given day to a commit hash (step 3).

If this tags are gone then I would have to rely instead on this command

$ ./wpt rev-list --epoch 1d --max-count=4
52b3307a5be5a03c10df2d96c96191575386be7b
d3422cc02be67fe0eaf5fe9df4c105a592d0d71c
7bb97b8a57ca84a972b7f34611fa73778adaf84d
824d27822a6c110981a6bbb99dddbcdf8455f56a

And pick the second entry (d3422cc02be67fe0eaf5fe9df4c105a592d0d71c) that would be the one mapping the run for the 12th of July.

So, not a big issue for me if this tags get removed as the ./wpt rev-list command is a good substitute

However my preference would be to automatically purge old tags (to avoid ending with thousands of tags) rather than stop tagging since I find convenient this tags.

But is not a strong preference in any case, feel free to push this if you think is better to remove the tags.

@clopez
Copy link

clopez commented Jul 12, 2022

The other potential option here would be to use a different namespace for the epoch refs, such as pushing them into refs/epochs/*.

That would be also a good option, i like it. It keeps the info for those interested on it (via git show-ref) meanwhile it hides this info from the default view of git log / git tag.

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.

5 participants