Skip to content

refactoring - #75

Open
techiealiraza wants to merge 129 commits into
developfrom
develop_new
Open

refactoring#75
techiealiraza wants to merge 129 commits into
developfrom
develop_new

Conversation

@techiealiraza

Copy link
Copy Markdown
Owner

No description provided.

techiealiraza and others added 30 commits June 2, 2023 18:47
Comment thread Gemfile
@@ -88,7 +81,6 @@ gem 'attr_encrypted', '~> 3.1'

gem 'daemons'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Ali -> Code Understanding

Comment thread Gemfile.lock Outdated
rack-protection (= 3.0.6)
tilt (~> 2.0)
smart_properties (1.17.0)
spring (4.1.1)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is not updated version ?

@@ -3,14 +3,15 @@
class ApplicationController < ActionController::Base
protect_from_forgery prepend: true

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

What is this ?

@@ -1,6 +1,5 @@
# frozen_string_literal: true

# Matches_Controller

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RESTFul Routes comments

index -> should have been a nested route tournaments/id/matches/

end
if @tournament.save
flash[:notice] = 'Tournament was successfully created.'
redirect_to tournament_url(@tournament)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

_path vs _url ?
Why url ?

redirect_to tournaments_url
else
flash[:errors] = @tournament.errors.full_messages.join(', ')
render :index

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

render -> index is wrong

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