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 #4853 - Remove Oxford (EGTK) holding point G #4856

Merged
merged 10 commits into from
Jul 25, 2023
Merged

Fixes #4853 - Remove Oxford (EGTK) holding point G #4856

merged 10 commits into from
Jul 25, 2023

Conversation

rishab-alt
Copy link
Collaborator

Fixes #4853

Summary of changes

remove Holding point G

@rishab-alt rishab-alt added the airac AIP-related changes label Jul 22, 2023
@rishab-alt rishab-alt added this to the 2308 milestone Jul 22, 2023
@rishab-alt rishab-alt self-assigned this Jul 22, 2023
Copy link
Collaborator

@PLM1995 PLM1995 left a comment

Choose a reason for hiding this comment

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

Please remove from the .kmz too

.github/CHANGELOG.md Outdated Show resolved Hide resolved
@rishab-alt
Copy link
Collaborator Author

Please remove from the .kmz too

DONE

Co-authored-by: Peter Mooney <[email protected]>
@PLM1995
Copy link
Collaborator

PLM1995 commented Jul 22, 2023

Please remove from the .kmz too

DONE

Ta, but I think you've inadvertantly updated an outdated version. This is a general problem in my view that we're keeping loads of different .kmz files, so a tiny change like this needs a new file/filename.

I propose (in general - not just for EGTK):

  • Going forward we have only one 'active' .kmz per SMR, named simply by the ICAO, e.g. EGTK.kmz which is updated for changes, rather than creating a new version each time minor changes/updates are made
  • Any existing older 'version' ones get archived in a folder within the _data\SMR Files\EGxx folder, maybe like _data\SMR Files\EGTK\Old\
  • If there is a major overhaul, like a redraw of the whole SMR, potentially consider archiving the older style, but not sure if this is necessary

Thoughts of others as ever welcome. @hazzas-99 et al

@rishab-alt Let's leave this PR as is for now until we have a definite plan how to handle the 5(!) Oxford .kmz files.

@AliceFord
Copy link
Collaborator

Please remove from the .kmz too

DONE

Ta, but I think you've inadvertantly updated an outdated version. This is a general problem in my view that we're keeping loads of different .kmz files, so a tiny change like this needs a new file/filename.

I propose (in general - not just for EGTK):

  • Going forward we have only one 'active' .kmz per SMR, named simply by the ICAO, e.g. EGTK.kmz which is updated for changes, rather than creating a new version each time minor changes/updates are made
  • Any existing older 'version' ones get archived in a folder within the _data\SMR Files\EGxx folder, maybe like _data\SMR Files\EGTK\Old\
  • If there is a major overhaul, like a redraw of the whole SMR, potentially consider archiving the older style, but not sure if this is necessary

Thoughts of others as ever welcome. @hazzas-99 et al

@rishab-alt Let's leave this PR as is for now until we have a definite plan how to handle the 5(!) Oxford .kmz files.

If you're suggesting not creating new versions of the .kmz unless we're making a big change (which I think you are), sounds good to me! We'd just have to be careful archiving 'old' versions as in some SMRs it seems the newest version isn't just called EGxx SMR, instead it's one with the airac.

@PLM1995
Copy link
Collaborator

PLM1995 commented Jul 23, 2023

We'd just have to be careful archiving 'old' versions as in some SMRs it seems the newest version isn't just called EGxx SMR, instead it's one with the airac.

Yes, should (hopefully) be fairly obvisous from the last edit date - assuming the correct one was the last editted one.

@AliceFord
Copy link
Collaborator

AliceFord commented Jul 23, 2023

We'd just have to be careful archiving 'old' versions as in some SMRs it seems the newest version isn't just called EGxx SMR, instead it's one with the airac.

Yes, should (hopefully) be fairly obvisous from the last edit date - assuming the correct one was the last editted one.

Mine are all dated at when I first cloned the repo D: (same for all other SMRs too)
image

@PLM1995
Copy link
Collaborator

PLM1995 commented Jul 23, 2023

Mine are all dated at when I first cloned the repo D: (same for all other SMRs too) image

Your local files might do that yeah, but https://github.com/VATSIM-UK/UK-Sector-File/tree/main/_data/SMR%20Files/EGLL for example will show the last commit dates to the actual repo, so yeah we'll have to be careful but it's not impossible :)

@hazzas-99
Copy link
Contributor

I'm very far removed from SMRs, haven't updated one in years. It struck me as a little odd that we were storing more than one kmz though? Surely if we need to revert to previous we can just access/download a previous version attached to a commit in the history? @PLM1995 @AliceFord

@PLM1995
Copy link
Collaborator

PLM1995 commented Jul 23, 2023

I'm very far removed from SMRs, haven't updated one in years. It struck me as a little odd that we were storing more than one kmz though? Surely if we need to revert to previous we can just access/download a previous version attached to a commit in the history? @PLM1995 @AliceFord

Yeah, exactly @hazzas-99. Seems easier to have one file we update than lots which don't track the changes well.

For this PR, it's probably easier for me to mess with the .kmz files myself @rishab-alt as explaining it would be beyond me and it isn't really directly related to this issue. I'll open a separate issue to standardise the others, but not start work on that until we have no open PRs editting .kmz files or we'll get conflicts when we try to merge.

@AliceFord would you like to document our policy on the wiki (I know how much you love the wiki) - feel free to run your changes by me first on discord.

New version based on most recent previous version, with G removed
Copy link
Collaborator

@PLM1995 PLM1995 left a comment

Choose a reason for hiding this comment

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

Having to approve this myself as had previously requested changes, happy I did it right @AliceFord? @rishab-alt

@PLM1995 PLM1995 requested a review from AliceFord July 23, 2023 09:08
@PLM1995 PLM1995 mentioned this pull request Jul 23, 2023
Copy link
Collaborator

@AliceFord AliceFord left a comment

Choose a reason for hiding this comment

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

All good 👍

Copy link
Collaborator

@PLM1995 PLM1995 left a comment

Choose a reason for hiding this comment

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

How didn't I spot these first time I don't know - No need for the blank lines

Airports/EGTK/SMR/Geo.txt Outdated Show resolved Hide resolved
Airports/EGTK/SMR/Labels.txt Outdated Show resolved Hide resolved
@PLM1995 PLM1995 merged commit 2de3786 into main Jul 25, 2023
3 checks passed
@PLM1995 PLM1995 deleted the issue-4853 branch July 25, 2023 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
airac AIP-related changes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Oxford (EGTK) Remove Holding Point G
4 participants