Opened 4 years ago

Closed 4 years ago

#22963 closed enhancement (fixed)

Unify TEST:: and TESTS::

Reported by: jmantysalo Owned by:
Priority: trivial Milestone: sage-8.0
Component: documentation Keywords:
Cc: tscrim Merged in:
Authors: Jori Mäntysalo Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: 05e6461 (Commits, GitHub, GitLab) Commit: 05e6461db05a68ced34f4ffa40f93c91b26371ae
Dependencies: Stopgaps:

Status badges

Description

This patch unifies TEST:: to TESTS:: to make it easier to automatically handle docstrings.

Change History (11)

comment:1 Changed 4 years ago by jmantysalo

  • Branch set to u/jmantysalo/test-tests

comment:2 Changed 4 years ago by jmantysalo

  • Cc tscrim added
  • Commit set to 05e6461db05a68ced34f4ffa40f93c91b26371ae
  • Status changed from new to needs_review

New commits:

05e6461From TEST:: to TESTS::

comment:3 follow-up: Changed 4 years ago by kcrisman

I don't see anything here alerting DEVELOPERS to this change, though I admit there are lots of files touched. It should be very clear how to format things in the developer guide.

Also, this should be announced (if not discussed) on sage-devel, if it hasn't been already. (I'm not suggesting this is a bad change, but just making sure people are aware - I stumbled upon this quite randomly.)

comment:4 Changed 4 years ago by kcrisman

(Also, unfortunately one might have to still root out some anyway, since currently-active tickets may not have this ...)

comment:5 in reply to: ↑ 3 Changed 4 years ago by jmantysalo

Replying to kcrisman:

I don't see anything here alerting DEVELOPERS to this change, though I admit there are lots of files touched. It should be very clear how to format things in the developer guide.

Also, this should be announced (if not discussed) on sage-devel, if it hasn't been already. (I'm not suggesting this is a bad change, but just making sure people are aware - I stumbled upon this quite randomly.)

It is hard for me to see who would be against this change. And there already is for example #21592 and #21652, and also see #21647. Should all those been discussed too?

comment:6 follow-up: Changed 4 years ago by kcrisman

One can argue all of those examples were "wrong", for some definition of wrong, while this one is presumably at least grammatically wrong.

But my actual rationale in posting is to suggest you make sure people know, so that you don't have to open 25 more tickets fixing this from people who didn't notice this otherwise-obscure ticket.

comment:7 in reply to: ↑ 6 Changed 4 years ago by kcrisman

Clarifying - unfortunately I replied and didn't click "edit".

One can argue all of those examples were fixing something "wrong", for some definition of wrong, while this one is presumably at least making some blocks grammatically wrong.

But my actual rationale in posting is to suggest you make sure people know, so that you don't have to open 25 more tickets fixing this from people who didn't notice this otherwise-obscure ticket.

Last edited 4 years ago by kcrisman (previous) (diff)

comment:8 Changed 4 years ago by jmantysalo

comment:9 follow-up: Changed 4 years ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

Makes sense for me. There are still many places with TEST: though. I guess you didn't think of that.

comment:10 in reply to: ↑ 9 Changed 4 years ago by kcrisman

Makes sense for me. There are still many places with TEST: though. I guess you didn't think of that.

Hence my mention of a patch bomb ... I guess that didn't happen yet.

comment:11 Changed 4 years ago by vbraun

  • Branch changed from u/jmantysalo/test-tests to 05e6461db05a68ced34f4ffa40f93c91b26371ae
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.