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

Remove static sectors #2701

Merged
merged 4 commits into from
May 4, 2021
Merged

Conversation

Fabien-B
Copy link
Contributor

As suggested in #2677, I propose here to remove static sectors. All sector will be dynamic ones.

fvantienen
fvantienen previously approved these changes Apr 30, 2021
Copy link
Member

@fvantienen fvantienen left a comment

Choose a reason for hiding this comment

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

I fully agree, this also removes the problem with the correct orientation.

@gautierhattenberger
Copy link
Member

I also agree, but maybe one little thing: we should keep the DTD unchanged for some time and print a warning with a deprecated message when using type. People with personal flight plans should have some time to fix them if needed.

@Fabien-B
Copy link
Contributor Author

I suggest making it an error, so people will take care of the problem before the DTD change.
Running now in Error: attribute "type" on tag sector is deprecated and must be removed is better than not paying attention to a warning, and running in a few weeks in a n obscure DTD error.

@fvantienen
Copy link
Member

I agree with the error, this forces people to update their files.

On a side note, shouldn't we list those deprecations somewhere with a specific enddate and a link to the specific code to remove/update. This reminds both the user and maintainers to update/remove this code. So my advice is to do the following:

  • Remove all code that is labeled 'deprecated' for more than 1 year or add them to the list
  • For new deprecations like this, add them to a list in a file somewhere with a specific enddate

@Fabien-B
Copy link
Contributor Author

Fabien-B commented May 4, 2021

Maybe we can use this github action, together with issues for that?
https://github.com/marketplace/actions/due-dates
And we can also set a "deprecation" tag to these issues so people can find them easily.

@fvantienen
Copy link
Member

For me that seems fine 👍 What do you think @gautierhattenberger ?

@gautierhattenberger
Copy link
Member

we can give a try until github have a proper "due date" option for pull requests

Here, we have also moved the definition of sectors outside the NAV_C part, so you can use that anywhere in the code and don't need the (dangerous) workaround to define NAV_C twice.

Copy link
Member

@fvantienen fvantienen left a comment

Choose a reason for hiding this comment

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

Seems fine 👍

@gautierhattenberger gautierhattenberger merged commit e965639 into paparazzi:master May 4, 2021
@Fabien-B Fabien-B deleted the sectors branch May 4, 2021 15:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants