On 11/14/23 16:48, Dumitru Ceara wrote:
> On 11/14/23 16:00, Ilya Maximets wrote:
>> On 11/14/23 13:31, Dumitru Ceara wrote:
>>> On 11/14/23 13:23, Ilya Maximets wrote:
>>>> On 11/14/23 13:12, Dumitru Ceara wrote:
>>>>> On 11/14/23 12:57, Ilya Maximets wrote:
>>>>>> On 11/14/23 12:52, Dumitru Ceara wrote:
>>>>>>> On 11/14/23 12:18, Ilya Maximets wrote:
>>>>>>>> These are useful to spot build failures in different CI systems.
>>>>>>>> For example, documentation build is failing for about 2 months
>>>>>>>> now, the badge might help with spotting the issue earlier.
>>>>>>>>
>>>>>>>> Signed-off-by: Ilya Maximets <[email protected]>
>>>>>>>> ---
>>>>>>>
>>>>>>> Awesome!  One minor comment below though.
>>>>>>>
>>>>>>>>  README.rst | 11 +++++++++++
>>>>>>>>  1 file changed, 11 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/README.rst b/README.rst
>>>>>>>> index 74ae22f41..6fb717742 100644
>>>>>>>> --- a/README.rst
>>>>>>>> +++ b/README.rst
>>>>>>>> @@ -5,6 +5,17 @@
>>>>>>>>  OVN
>>>>>>>>  ===
>>>>>>>>  
>>>>>>>> +.. image:: 
>>>>>>>> https://github.com/ovn-org/ovn/actions/workflows/test.yml/badge.svg
>>>>>>>> +    :target: https://github.com/ovn-org/ovn/actions/workflows/test.yml
>>>>>>>> +.. image:: 
>>>>>>>> https://github.com/ovn-org/ovn/actions/workflows/ovn-kubernetes.yml/badge.svg
>>>>>>>> +    :target: 
>>>>>>>> https://github.com/ovn-org/ovn/actions/workflows/ovn-kubernetes.yml
>>>>>>>> +.. image:: 
>>>>>>>> https://github.com/ovn-org/ovn/actions/workflows/ovn-fake-multinode-tests.yml/badge.svg
>>>>>>>> +    :target: 
>>>>>>>> https://github.com/ovn-org/ovn/actions/workflows/ovn-fake-multinode-tests.yml
>>>>>>>> +.. image:: https://api.cirrus-ci.com/github/ovn-org/ovn.svg
>>>>>>>> +    :target: https://cirrus-ci.com/github/ovn-org/ovn
>>>>>>>> +.. image:: https://readthedocs.org/projects/ovn/badge/?version=latest
>>>>>>>> +    :target: https://docs.ovn.org/en/latest/
>>>>>>>
>>>>>>> Shouldn't this point to https://readthedocs.org/projects/ovn/builds/
>>>>>>> instead?  Like that we'd get redirected to the most recently executed
>>>>>>> builds instead of the last successful docs page.
>>>>>>
>>>>>> Read The Docs suggests to redirect to the built docs, i.e. it generates
>>>>>> a badge code this way.  And it make some sense that the badge that says
>>>>>> 'docs' leads to the project documentation.  But I'll leave it up to you.
>>>>>>
>>>>>
>>>>> I see now that that's the way Read The Docs generates the badge by
>>>>> default.  However, a contributor:
>>>>
>>>> I think, the main point here is that the link is not for a contributor,
>>>> but for the user who reads the README looking for documentation, so the
>>>> 'docs' button intuitively leads to the project docs, not to the docs build
>>>> status page.
>>>>
>>>
>>> Fair point.
>>>
>>>>> a. notices the badge is red
>>>>> b. clicks on the badge link
>>>>> c. gets redirected to https://docs.ovn.org/en/latest/?badge=latest
>>>>> d. AFAICT there's no link there to point to the failing build
>>>>> (https://readthedocs.org/projects/ovn/builds/)
>>>>>
>>>>> We could probably mention the OVN Read The Docs URL in the
>>>>> documentation.rst file but that's not always easy to notice.  I think
>>>>> I'd still prefer the link in the badge to take us to the most recently
>>>>> executed builds.  It would also match what happens for the other badges
>>>>> this patch is adding.
>>>>
>>>> Quick search on GitHub shows that most projects follow the badge format
>>>> suggested by RTD, i.e. point to the project documentation, instead of
>>>> the docs build status page.  So, changing that may have more friction
>>>> for new users familiar with other projects using RTD.
>>>>
>>>> Not a strong opinion from my side, if you still prefer the build status
>>>> page, I can change the link.
>>>
>>> Let's leave it as is but can we add a link in documentation.rst to point
>>> us to the executed builds?
>>
>> Sounds like a separate change. :)
>>
> 
> OK :)
> 
> Acked-by: Dumitru Ceara <[email protected]>
> 
> 

I went ahead and applied this to main.  Now we have badges showing how
our CI is failing - that is actually a very good thing! - let's fix the
CI next.. :)

Regards,
Dumitru

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to