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

Fixes all remaining found issues with last nickname commit, Also impliements finalised version of PR 1163 #1164

Closed
wants to merge 8 commits into from

Conversation

hitime1234
Copy link
Contributor

Changes:
removes and adds fixes to rest of issues found see:
#1159
#1154
for more detail

Other changes impliemented a finalised version of
#1153

365391425-1be26c26-fd56-4c8d-b24e-c7626f143906

Let me know if I missed any

Squashed and move to new branch to so history doesn't get wack :3

docs/install.md Outdated
@@ -29,8 +29,7 @@ If you have Docker on your system, use Docker Compose to set up an environment.
Simply run `docker compose up -d` and visit "https://localhost:4443/myradio/".

If you encounter an error with autoload.php:
find the id of your myradio container using `docker dontainer ls`
enter a bash session with `docker exec -it [myradioid] bash`
enter a bash session with `docker compose exec myradio bash`
Copy link
Member

Choose a reason for hiding this comment

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

more concise to do docker compose exec myradio composer install

Copy link
Contributor Author

Choose a reason for hiding this comment

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

should be changed

}
],
"bPaginate": true
});
});
Copy link
Member

Choose a reason for hiding this comment

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

this should be a separate PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed will add to seperate pr

@@ -1,3 +1,14 @@
//Name helper function for concating names;
function NameHelper(data) {
Copy link
Member

Choose a reason for hiding this comment

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

maybe a better name for this (formatName? or something along those lines)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Change name to formatName as that works

@@ -442,7 +442,7 @@ public function getFName()
}

/**
* Returns the User's first name.
* Returns the User's nickname.
*
* @return string The User's first name
Copy link
Member

Choose a reason for hiding this comment

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

this still needs changing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

whoops fixed it

Copy link
Member

@ashhhleyyy ashhhleyyy left a comment

Choose a reason for hiding this comment

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

docs/install.md Show resolved Hide resolved
docs/install.md Show resolved Hide resolved
@@ -1,3 +1,14 @@
//Format names function for concating names;
Copy link
Member

Choose a reason for hiding this comment

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

not sure the comment is really necessary, and it reads quite confusingly to me

Copy link
Contributor Author

@hitime1234 hitime1234 Sep 13, 2024

Choose a reason for hiding this comment

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

//combines the first ,nickname, last names of the user together
would this be good or show I remove the comment fully

Copy link
Member

Choose a reason for hiding this comment

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

probably best to just yeet it

}
return $this->fname.' '.$this->nname.' '.$this->sname;
}
return $this->fname.' "'.$this->nname.'" '.$this->sname;
Copy link
Member

@ashhhleyyy ashhhleyyy Sep 15, 2024

Choose a reason for hiding this comment

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

it feels like this if case should be inverted, as having a nickname set is definitely the non-default case

@alyxbb alyxbb closed this Oct 3, 2024
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.

3 participants