LINUX.ORG.RU
ФорумTalks

Про LOR-API и реквесты которые никто не принимает.

 ,


0

1

Вот я взял и посмотрел на «невинно не принятый» пулл:

https://github.com/maxcom/lorsource/pull/642/files

From 03b8cfd31484bb95cfb600a77755b7794c928694 Mon Sep 17 00:00:00 2001
From: 
Date: Tue, 2 Jun 2015 12:39:50 +0300
Subject: [PATCH] Add "/api" prefixes

---
 src/main/java/ru/org/linux/user/UserEventApiController.java | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/main/java/ru/org/linux/user/UserEventApiController.java b/src/main/java/ru/org/linux/user/UserEventApiController.java
index a15c68e..a9291cf 100644
--- a/src/main/java/ru/org/linux/user/UserEventApiController.java
+++ b/src/main/java/ru/org/linux/user/UserEventApiController.java
@@ -35,7 +35,7 @@
   private UserEventService userEventService;
 
   @ResponseBody
-  @RequestMapping(value = "/notifications-count", method= RequestMethod.GET)
+  @RequestMapping(value = "/api/notifications-count", method= RequestMethod.GET)
   public int getEventsCount(HttpServletRequest request, HttpServletResponse response) throws Exception {
     Template tmpl = Template.getTemplate(request);
     if (!tmpl.isSessionAuthorized()) {
@@ -47,7 +47,7 @@ public int getEventsCount(HttpServletRequest request, HttpServletResponse respon
     return tmpl.getCurrentUser().getUnreadEvents();
   }
 
-  @RequestMapping(value="/notifications-reset", method = RequestMethod.POST)
+  @RequestMapping(value="/api/notifications-reset", method = RequestMethod.POST)
   @ResponseBody
   public String resetNotifications(
     HttpServletRequest request,
@@ -66,7 +66,7 @@ public String resetNotifications(
   }
 
   @ResponseBody
-  @RequestMapping(value = "/yandex-tableau", method = RequestMethod.GET, produces={"application/json"})
+  @RequestMapping(value = "/api/yandex-tableau", method = RequestMethod.GET, produces={"application/json"})
   public Map<String, Integer> getYandexWidget(HttpServletRequest request, HttpServletResponse response) throws Exception {
     Template tmpl = Template.getTemplate(request);
     if (!tmpl.isSessionAuthorized()) {
@@ -78,7 +78,7 @@ public String resetNotifications(
     }
   }
 
-  @RequestMapping(value = "/notifications-list", method = RequestMethod.GET, produces = "application/json; charset=UTF-8")
+  @RequestMapping(value = "/api/notifications-list", method = RequestMethod.GET, produces = "application/json; charset=UTF-8")
   @ResponseBody
   public Map<String, Object> listNotifications(
           @RequestParam(value = "filter", required = false) String filterAction,

Для тех кто не понял, скажу, коммит ломает обратную совместимость с имеющимися кодом, причем «просто так».

Сломано просто так, блиан! На адекватное работе за такое эльфийское поведение делают предупредительную прочистку мозгов, если не помогло то увольняют.

С подобным я встречался в своей практике, только тогда мне бедняжко не смогло мотивировать свое поведение, может тут на лоре есть кто-то кто просветит?

Deleted

Попробуй спросить автора PR. Может он таки в состоянии объяснить.

Virtuos86 ★★★★★ ()

Это адреса, на которые навешиваются обработчики? Если так, у этого API еще нет пользователей, о какой совместимости тогда говорить?

И вроде даже хорошо, что выносят его в отдельную секцию /api, чтобы не путалось с остальными адресами. м?

staseg ★★★★★ ()
Последнее исправление: staseg (всего исправлений: 1)

Для тех кто не понял, скажу, коммит ломает обратную совместимость с имеющимися кодом, причем «просто так».

1) У вещей, которыми никто не пользуется.

2) Не просто так, а для единообразия (он ведь всё в api/ выносит?).

aidan ★★★★ ()

Нет публичного интерфейса == нет гарантий совместимости.

Deleted ()

Не вижу ничего плохого в этом коммите - оно ведь ещё не зарелизилось. А пулл-реквест же не приняли, ты сам написал, тогда в чём ваще проблема? А так ваще по коду, я бы вынес адреса в отдельные именованные константы в одно место и использовал константы, вместо того, чтобы «сырые» строки разбрасывать по коду - в одном месте поменял, в другом забыл, всё накрылось.

DELIRIUM ☆☆☆☆☆ ()
Последнее исправление: DELIRIUM (всего исправлений: 1)

Так именно потому они и висят.

CYB3R ★★★★★ ()

может тут на лоре есть кто-то кто просветит?

Очень просто, в devel версии это допустимо. А что, у API уже был финальный релиз?

Flame4All ()
Ответ на: комментарий от DELIRIUM

+100. и вот за такие упущения надо бить, а нытье в оппосте ни о чем.

dib2 ★★★★★ ()
Ответ на: комментарий от staseg

Если так, у этого API еще нет пользователей,

хоть один бы чудак, хотябы посмотрел к ками урл-ам обращается браузер при загрузке этого топика, ппц. Ладно форум, но 99% остальных людей такиеже, берутся отвечать нихрена не пытавшись разобраться, но с умным видом.

Deleted ()
Ответ на: комментарий от Deleted

Только вот в адекватных фреймворках, а не в говне из 90-х, есть инструменты против хардкода урлов:

https://docs.djangoproject.com/en/1.8/topics/http/urls/#reverse-resolution-of...

http://flask.pocoo.org/docs/0.10/quickstart/#url-building

http://api.rubyonrails.org/classes/ActionDispatch/Routing/UrlFor.html

Deleted ()
Ответ на: комментарий от Deleted

Вот ты странный, сначала завел разговор для «тех кто не понял», а теперь я уже разбираться должен? Ну ок, ты знаешь больше всех, можешь порадоваться и успокоиться.

staseg ★★★★★ ()
Ответ на: комментарий от DELIRIUM

Не вижу ничего плохого в этом коммите - оно ведь ещё не зарелизилось.

(Я считаю, что навес над велосипедами должен быть из нержавеющей стали).

Состояния между изменениями должны быть по возможности рабочими. Если система работала до патча, она должна работать и после патча. Точно так же нельзя ломать патчами сборку.

i-rinat ★★★★★ ()
Ответ на: комментарий от i-rinat

о свет во тьме 8)

еще пулл реквест должен затрагивать только заявленный минимум кода, а не «а тут я еще размер шрифта подправил», а тут «добавил метод апи» а по факту еще сломал остальное

Deleted ()
Ответ на: комментарий от Deleted

У тебя что жаба-программисты левую ручку увели?

Deleted ()
Ответ на: комментарий от Deleted

Ты правда думал, что все полезут разбираться, как работает ЛОР? Сказал «А» – ломает совместимость, говори и «Б» – ЛОР уже сейчас использует это API. Из кода это совсем не очевидно же.

staseg ★★★★★ ()
Ответ на: комментарий от staseg

Блаженное чудо «ломает совместимость» - значит ломает, если тебе вдруг приснилось, что речь идет о неиспользуемом апи то сам оторви свой жирный зад и озадачься исследованиями, а пока ты себе там чего-то выдумал и типа тебе кто-то должен

Deleted ()
Ответ на: комментарий от Flame4All

Разве?

До:

www.linux.org.ru/notifications-count — используется и работает;
www.linux.org.ru/api/notifications-count — не используется и не работает.

После:

www.linux.org.ru/notifications-count — используется и не работает;
www.linux.org.ru/api/notifications-count — работает, но не используется.

i-rinat ★★★★★ ()
Ответ на: комментарий от Deleted

Да-да, а ещё программист делает тесты и запускает тесты перед тем, как пушить. Теперь это несбывшихся-надежд-тред.

i-rinat ★★★★★ ()
Последнее исправление: i-rinat (всего исправлений: 1)
Ответ на: комментарий от i-rinat

После:
www.linux.org.ru/notifications-count — используется и не работает;
www.linux.org.ru/api/notifications-count — работает, но не используется.

Работает: http://i.imgur.com/K5BSMiJ.png

Если ты про API, то релиза не было вроде, оно ещё в разработке, могут ломать сколько угодно.

Flame4All ()
Ответ на: комментарий от Flame4All

Кстати, 4.2, первое выдаёт результат, второе 404.

Ты что, развернул у себя локальную копию? Естественно, на самом сайте сейчас вариант «до».

i-rinat ★★★★★ ()
Ответ на: комментарий от Deleted

Оно _сейчас_ работает по старому урлу, после принятия коммита по старому урлу _будет_ тыква, а тебя надо гнать ссаными тряпками из it за такую тупизну.

Тупишь ты. Или хочешь сказать на сервере движок ЛОРа обновляют сразу после каждого коммита в гитхаб? Или всё таки работающая версия на сервере ЛОРа ≠ версии на гитхабе?

Flame4All ()
Ответ на: комментарий от i-rinat

Ты что, развернул у себя локальную копию? Естественно, на самом сайте сейчас вариант «до».

Тогда о чём батхёрт? На гитхабе devel версия, на сервере стабильная версия.

Flame4All ()
Ответ на: комментарий от Deleted

Когда я был выфером для таких как ты я придумал специальны термин «амозглики».

Да ты и сейчас всё тот же дурачок.

Flame4All ()
Ответ на: комментарий от Flame4All

Если ты про API, то релиза не было вроде, оно ещё в разработке, могут ломать сколько угодно.

Оспадетыбожемой.

Давай я объясню тебе ещё раз. Речь идёт про pull-request, который переименовывает некоторые URL в одних местах, но оставляет их старыми в остальных. Это даже не «мы поломали в одном коммите, но исправили в другом». Это просто четыре патча (вместо одного), которые тупо ломают сайт. Никаких фич.

i-rinat ★★★★★ ()
Ответ на: комментарий от i-rinat

которые тупо ломают сайт.

Сайт пока никто не сломал, он работает на своей версии, которую обновят когда в git будет всё готово. Я думаю в git урлы поправят другими коммитами позже, не дураки. Ну что вы как дети...

Flame4All ()
Ответ на: комментарий от Flame4All

Я думаю в git урлы поправят другими коммитами позже

Я надеюсь, что ты не участвуешь в разработке хоть какого-то софта, которым я так или иначе пользуюсь.

i-rinat ★★★★★ ()
Ответ на: комментарий от Deleted

Потому что Flame4All и прочим авторам подобных коммитов, что-то объяснять бессмысленно. Не, еслиб он был бы моим подчиненным то яб потратил немного времени на душеспасительные беседы, на моем опыте это иногда помогало, но в общем случае проще закопать.

Deleted ()
Ответ на: комментарий от Flame4All

Я думаю в git урлы поправят другими коммитами позже, не дураки.

не дураки просто не приняли это говно, а ты бы принял, ибо тупишь.

Deleted ()
Ответ на: комментарий от i-rinat

Ну а как ты представляешь разработку? То что в разрабатываемой ветке что-то ломают в процессе, это нормально. Я не в курсе, есть ли у лоровского движка стабильная ветка, если нет, значит у гневно ещё не было официального релиза.

Flame4All ()
Ответ на: комментарий от i-rinat

судя по коментам в треде, не он - так аналогов полно, хорошо руководители проектов, как правило, ребята опытные и вычищают это из команды

ну а если в проекте все такие то это хорошо заметно по регулярным факапам

Deleted ()
Ответ на: комментарий от Deleted

Блаженное чудо «ломает совместимость» - значит ломает, если тебе вдруг приснилось, что речь идет о неиспользуемом апи то сам оторви свой жирный зад и озадачься исследованиями, а пока ты себе там чего-то выдумал и типа тебе кто-то должен

Вот я прям щас пойду исследовать каждый высер наркомана.

Кончай юлить. Ты дал сумбурное описание, посреди топика соизволил пояснить, что же ты имел в виду на самом деле, и продолжаешь гнуть пальцы. Кто виноват, что ты не осилил сразу описать проблему по существу?

ЗЫ. В остальном я с тобой согласен, коммитеру строгий выговор.

staseg ★★★★★ ()
Ответ на: комментарий от Deleted

не дураки просто не приняли это говно, а ты бы принял, ибо тупишь.

А, так его ещё не приняли. Выфер, ты всё та же стервозная истеричка. Это опенсорс, коммиты слать может любой дурак. Если ты так любишь истерить, в интернетах, куда доступ сейчас есть всем, ты можешь для этого найти полно поводов. Зачем только это тащить сюда?

Flame4All ()
Ответ на: комментарий от Deleted

Мне жалко людей, которым приходится с тобой иметь дело IRL... Как и тех, у кого начальник стервозная баба.

Flame4All ()
Ответ на: комментарий от Flame4All

То что в разрабатываемой ветке что-то ломают в процессе, это нормально.

Пока эта ветка у тебя на локальной машине или в личном репозитории, поступай как хочешь. Но к моменту pull-request'а всё нужно причесать. Если возможно, код нужно разбить на отдельные патчи, решающие части задачи. При этом каждое промежуточное состояние должно собираться и работать. Иногда для этого нужно не только разбивать один патч на несколько, но и сливать несколько патчей в один.

Просто представь себе, что тебе нужно найти бисекцией патч, из-за которого появилась регрессия. Создавай коммиты так, чтобы это было легко.

Твои патчи будет просматривать человек, которому твои изменения не очень-то и нужны. Уж постарайся сделать так, чтобы они его не бесили.

Я не в курсе, есть ли у лоровского движка стабильная ветка, если нет, значит у гневно ещё не было официального релиза.

Ткнуть на ссылку в заглавном посте, открыть главную страницу репозитория, ткнуть в список веток и поглядеть. Времени занимает меньше, чем написать фразу о том, что ты не в курсе.

Ветка master — и есть стабильная ветка.

i-rinat ★★★★★ ()
Вы не можете добавлять комментарии в эту тему. Тема перемещена в архив.